[PATCH] shrink last_char_is function even more

Tito farmatito at tiscali.it
Thu Jul 9 19:13:51 UTC 2020


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?

Ciao,
Tito

> On Tue, 7 Jul 2020 at 14:17, Tito <farmatito at tiscali.it <mailto:farmatito at tiscali.it>> 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
>     busybox at busybox.net <mailto:busybox at busybox.net>
>     http://lists.busybox.net/mailman/listinfo/busybox
> 


More information about the busybox mailing list