Re: [PATCH 4/4] tools/nolibc: add missing memchr() to string.h

[Willy Tarreau <w@1wt.eu>]

On Sat, Jun 21, 2025 at 10:27:11AM +0200, Thomas Weißschuh wrote:
> On 2025-06-20 12:02:51+0200, Willy Tarreau wrote:
> > Surprisingly we forgot to add this common one. It was added with a
> > per-arch guard allowing to later implement it in arch-specific asm
> > code like was done for a few other ones.
> > 
> > The test verifies that we don't search past the indicated length.
> > 
> > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > ---
> >  tools/include/nolibc/string.h                | 15 +++++++++++++++
> >  tools/testing/selftests/nolibc/nolibc-test.c |  2 ++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/tools/include/nolibc/string.h b/tools/include/nolibc/string.h
> > index 163a17e7dd38b..4000926f44ac4 100644
> > --- a/tools/include/nolibc/string.h
> > +++ b/tools/include/nolibc/string.h
> > @@ -93,6 +93,21 @@ void *memset(void *dst, int b, size_t len)
> >  }
> >  #endif /* #ifndef NOLIBC_ARCH_HAS_MEMSET */
> >  
> > +#ifndef NOLIBC_ARCH_HAS_MEMCHR
> 
> So far we only have added these guards when necessary,
> which they aren't here. Can we drop them?

I intentionally placed them so that we can easily override them,
as we did for the other ones on x86 where string operations are
super short (repnz scasb is two bytes once you have the registers
already loaded).

> > +static __attribute__((unused))
> > +void *memchr(const void *s, int c, size_t len)
> > +{
> > +	char *p = (char *)s;
> 
> The docs say that they are interpreted as "unsigned char".

It does not change anything here, except adding an extra
modifier (since we'll then also have to do it in the loop
when comparing against c), thus IMHO it's extra noise.

> Also, can we keep the const?

It's memchr()'s definition which requires to return a void* so the
const needs to be dropped somewhere. Here I found visually cleaner to
have a single cast during the variable assignment rather than have a
second one on the return statement. But it's a matter of taste. I
tend to hate casts as they confuse the reader and remove the ability
of the compiler to produce relevant warnings, so for me the less the
better.

Thanks,
Willy