[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