On 7/9/20 9:13 PM, Tito wrote: > On 7/9/20 9:56 PM, Martin Lewis wrote: >> Please note that my original patch is still smaller: >> http://lists.busybox.net/pipermail/busybox/2020-June/088026.html > > Hi, > yes I know. This is smaller than what is in git now. > I understood that Denis rejected your patch: > "This scans the string twice, unnecessarily. Let's not do that." > Can't say If he will like mine. > >> I'm not sure whether it's faster, it would be interesting to compare them. > > Can you suggest how could this be achieved? a test program?
Hi, by some rough test i did your version is way faster: real 0m0.005s user 0m0.006s sys 0m0.002s vs real 0m0.095s user 0m0.102s sys 0m0.001s This is on 10000 iterations on a 3000 char array. Ciao, Tito > Ciao, > Tito > >> On Tue, 7 Jul 2020 at 14:17, Tito <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi, >> attached you will find a patch that shrinks libbb's last_char_is >> function. >> bloatcheck is: >> >> function old new delta >> last_char_is 53 42 -11 >> >> ------------------------------------------------------------------------------ >> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-11) Total: -11 >> bytes >> text data bss dec hex filename >> 981219 16907 1872 999998 f423e busybox_old >> 981208 16907 1872 999987 f4233 busybox_unstripped >> >> Patch for review: >> >> --- libbb/last_char_is.c.orig 2020-07-05 09:54:24.737931000 +0200 >> +++ libbb/last_char_is.c 2020-07-06 14:29:27.768898574 +0200 >> @@ -11,14 +11,7 @@ >> /* Find out if the last character of a string matches the one given */ >> char* FAST_FUNC last_char_is(const char *s, int c) >> { >> - if (s) { >> - size_t sz = strlen(s); >> - /* Don't underrun the buffer if the string length is 0 */ >> - if (sz != 0) { >> - s += sz - 1; >> - if ((unsigned char)*s == c) >> - return (char*)s; >> - } >> - } >> - return NULL; >> -} >> + if (!s || !*s) return NULL; >> + while (*(s + 1)) s++; >> + return (*s == c) ? (char *) s : NULL; >> +} >> >> >> The alternative version: >> >> char* FAST_FUNC last_char_is(const char *s, int c) >> { >> if (!s || !*s) return NULL; >> while (*(++s)); >> return (*(--s) == c) ? (char *)s : NULL; >> } >> >> that was also posted to the list on my system was bigger: >> >> function old new delta >> last_char_is 53 46 -7 >> >> ------------------------------------------------------------------------------ >> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-7) Total: -7 >> bytes >> >> Ciao, >> Tito >> >> >> P.S.: test program, if you are aware of other corner cases please tell >> me: >> >> ____________________________________________________________________________ >> >> char* last_char_is(const char *s, int c) >> { >> if (!s || !*s) >> return NULL; >> while (*(s + 1)) >> s++; >> return (*s == c) ? (char *) s : NULL; >> } >> >> int main (int argc, char **argv) { >> char *c; >> int ret = 0; >> >> printf("Test 1 'prova','a' : "); >> c = last_char_is("prova", 'a'); >> if (c != NULL && *c == 'a') { >> puts("PASSED"); >> } else { >> puts("FAILED"); >> ret |= 1; >> } >> printf("Test 2 '' ,'x' : "); >> c = last_char_is("", 'x'); >> if (c != NULL) { >> puts("FAILED"); >> ret |= 1; >> >> } else { >> puts("PASSED"); >> } >> printf("Test 3 NULL ,'3' : "); >> c = last_char_is(NULL, 'e'); >> if (c != NULL) { >> puts("FAILED"); >> ret |= 1; >> } else { >> puts("PASSED"); >> } >> printf("Test 4 'prova','x' : "); >> c = last_char_is("prova", 'x'); >> if (c != NULL) { >> puts("FAILED"); >> ret |= 1; >> } else { >> puts("PASSED"); >> } >> printf("Test 5 'prova','\\n': "); >> c = last_char_is("prova", '\n'); >> if (c != NULL) { >> puts("FAILED"); >> ret |= 1; >> } else { >> puts("PASSED"); >> } >> printf("Test 6 'prova','\\0': "); >> c = last_char_is("prova", 0); >> if (c != NULL) { >> puts("FAILED"); >> ret |= 1; >> } else { >> puts("PASSED"); >> } >> return ret; >> } >> >> ____________________________________________________________________________________________ >> _______________________________________________ >> busybox mailing list >> [email protected] <mailto:[email protected]> >> http://lists.busybox.net/mailman/listinfo/busybox >> > _______________________________________________ > busybox mailing list > [email protected] > http://lists.busybox.net/mailman/listinfo/busybox > _______________________________________________ busybox mailing list [email protected] http://lists.busybox.net/mailman/listinfo/busybox
