You're obviously right. I didn't look at it as a diff. Sorry.
I would like to offer this (somewhat tested) code instead:
char* FAST_FUNC last_char_is(char *s, char c)
{
if (s && *s) {
while (*s != '\0') s++;
s--;
if (*s == c) return s;
}
return NULL;
}
For gcc -O2 (strlen="old", strrchr version blank, above="new"):
text data bss dec hex filename
91 0 0 91 5b last_char_is.o
97 0 0 97 61 last_char_is_new.o
139 0 0 139 8b last_char_is_old.o
for gcc -Os:
text data bss dec hex filename
78 0 0 78 4e last_char_is.o
85 0 0 85 55 last_char_is_new.o
105 0 0 105 69 last_char_is_old.o
The above code should be more efficient than both the strlen and strrchr
versions while still being smaller than the current strlen version. It
only scans the string once, doesn't do any unnecessary work, and rejects
both null and empty string inputs. I am not certain of the consequences
of dropping 'const' from the function, but it looks like the function
would be making a new copy of a pointer variable to work with anyway.
I wrote a version that calls strlen() but it was larger. All my tests
were done on x86_64.
On 2020-07-01 09:34, Martin Lewis wrote:
Notice that there's a "-" before the line with strlen.
Here's the final implementation to simplify reading:
char* FAST_FUNC last_char_is(const char *s, int c)
{
if (s) {
char *index = strrchr(s, c);
if (index && *(index + 1) == '\0')
return index;
}
return NULL;
}
On Tue, 30 Jun 2020 at 10:59, Jody Bruchon <j...@jodybruchon.com
<mailto:j...@jodybruchon.com>> wrote:
strlen scans the string; strrchr also scans the string.
On 2020-07-01 10:23, Martin Lewis wrote:
> Hi,
>
> I'm not sure what I'm missing, from a quick glance at strrchr
> (glibc's) it seems that it scans the string only once?
>
> Thank you,
> Martin
>
> On Tue, 30 Jun 2020 at 01:34, Denys Vlasenko
<vda.li...@googlemail.com <mailto:vda.li...@googlemail.com>
> <mailto:vda.li...@googlemail.com
<mailto:vda.li...@googlemail.com>>> wrote:
>
> This scans the string twice, unnecessarily. Let's not do that.
>
> On Thu, Jun 11, 2020 at 3:45 PM Martin Lewis
> <martin.lewis....@gmail.com
<mailto:martin.lewis....@gmail.com>
<mailto:martin.lewis....@gmail.com
<mailto:martin.lewis....@gmail.com>>>
> wrote:
> >
> > function old new
> delta
> > last_char_is 53 30
> -23
> >
>
------------------------------------------------------------------------------
> > (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-23) Total:
> -23 bytes
> > text data bss dec hex filename
> > 981322 16915 1872 1000109 f42ad busybox_old
> > 981299 16915 1872 1000086 f4296 busybox_unstripped
> >
> > Signed-off-by: Martin Lewis <martin.lewis....@gmail.com
<mailto:martin.lewis....@gmail.com>
> <mailto:martin.lewis....@gmail.com
<mailto:martin.lewis....@gmail.com>>>
> > ---
> > libbb/last_char_is.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/libbb/last_char_is.c b/libbb/last_char_is.c
> > index 66f2e3635..1fff08f9f 100644
> > --- a/libbb/last_char_is.c
> > +++ b/libbb/last_char_is.c
> > @@ -13,11 +13,10 @@
> > */
> > char* FAST_FUNC last_char_is(const char *s, int c)
> > {
> > - if (s && *s) {
> > - size_t sz = strlen(s) - 1;
> > - s += sz;
> > - if ( (unsigned char)*s == c)
> > - return (char*)s;
> > + if (s) {
> > + char *index = strrchr(s, c);
> > + if (index && *(index + 1) == '\0')
> > + return index;
> > }
> > return NULL;
> > }
> > --
> > 2.11.0
> >
> > _______________________________________________
> > busybox mailing list
> > busybox@busybox.net <mailto:busybox@busybox.net>
<mailto:busybox@busybox.net <mailto:busybox@busybox.net>>
> > http://lists.busybox.net/mailman/listinfo/busybox
>
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net <mailto:busybox@busybox.net>
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net <mailto:busybox@busybox.net>
http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox