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

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

On Sun, Jun 22, 2025 at 09:56:35PM +0200, Thomas Weißschuh wrote:
(...)
> > > > +#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).
> 
> Okay.
> 
> We do have different override mechanisms.
> Both NOLIBC_ARCH_HAS_* and for example the mechanism for sys_fork.
> Not sure if it is worth aligning them.

I don't know either, because we're speaking about doing it with standard
name functions (e.g. memchr), contrary to sys_* that we're bringing with
nolibc. I think it requires a bit more thinking to be sure we're not going
to cause trouble (e.g. with compiler builtin ones etc). At least whatever
the outcome, I agree that trying to align all definitions using the same
approach would be desirable, even if it means changing all of them.

> > > > +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.
> 
> Fair enough.
> 
> > > 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.
> 
> Ditto.

OK, then I'll push it.

Thank you!
Willy