[PATCH] Optimise trim_string() front-trimming

Martin Schwenke martin at meltin.net
Sun Jun 19 22:36:27 UTC 2016


On Sun, 19 Jun 2016 13:09:47 +0200, Ralph Boehme <slow at samba.org> wrote:

> On Sun, Jun 19, 2016 at 08:20:03PM +1000, Martin Schwenke wrote:
> > On Fri, 26 Jun 2015 16:13:51 -0700, Herb Lewis <hlewis at panasas.com>
> > wrote:
> >   
> > > We really should fix trim_string as well. The front trimming calls
> > > memmove for each character/string found instead of finding
> > > the first non-match and then calling memmove once.  
> > 
> > Like this?  Sometimes it is worth saving easy suggestions and
> > attacking them when fighting a cold...  :-)  
> 
> :) Get well soon!

Danke!

> > Attached patches:
> > 
> > 1. Add some tests for trim_string()  
> 
> rb: me.
> 
> > 2. Optimise trim_string()'s front trimming to call memmove(3) just once  
> 
> I think the len check can be removed in the while loop as it never
> changes and maybe move the strlen() call right after the check that
> ensures len will be > 0.
> 
> @@ -804,16 +804,15 @@ _PUBLIC_ bool trim_string(char *s, const char
> *front, const char *back)
>         /* Ignore null or empty strings. */
>         if (!s || (s[0] == '\0'))
>                 return false;
> +       len = strlen(s);
>  
>         front_len       = front? strlen(front) : 0;
>         back_len        = back? strlen(back) : 0;
>  
> -       len = strlen(s);
> -
>         if (front_len) {
>                 size_t front_trim = 0;
>  
> -               while (len && strncmp(s+front_trim, front, front_len)==0) {
> +               while (strncmp(s+front_trim, front, front_len)==0) {
>                         front_trim += front_len;
>                 }
>                 if (front_trim > 0) {
> 
> If you agree, feel free to push with this change.

Thanks for spotting that.  Sold!  :-)

Given that the "len = ..." line was being moved up, I also added
redundant braces around the "return false" as per the coding style.
There are a bunch of whitespace cleanups possible but I decided to
leave them for now...

Pushed.

peace & happiness,
martin



More information about the samba-technical mailing list