[PATCH 0/4] tools/nolibc: assorted fixes and small updates

[PATCH 0/4] tools/nolibc: assorted fixes and small updates

[Willy Tarreau]

Trying to port a program to nolibc triggered a few trivial errors that
are worth being addressed:
  - inttypes.h is missing, while being generally recommended over stdint.h
    for being more portable. Here we add it, which simply includes stdint.h.

  - sys/select.h is supposed to be where struct fd_set and FD_CLR() etc
    are defined. Till now it was still in types.h. Let's create the file
    and move these definitions there.

  - memchr() was still missing, thus a trivial one was added.

Finally the help message from "make help" reported an inaccurate
installation path, omitting the dependency on $OUTPUT. This was
fixed as well.

Willy Tarreau (4):
  tools/nolibc: fix misleading help message regarding installation path
  tools/nolibc: add the more portable inttypes.h
  tools/nolibc: move FD_* definitions to sys/select.h
  tools/nolibc: add missing memchr() to string.h

 tools/include/nolibc/Makefile                |  4 +-
 tools/include/nolibc/inttypes.h              |  8 +++
 tools/include/nolibc/string.h                | 15 ++++++
 tools/include/nolibc/sys/select.h            | 56 ++++++++++++++++++++
 tools/include/nolibc/types.h                 | 48 +----------------
 tools/testing/selftests/nolibc/nolibc-test.c |  2 +
 6 files changed, 85 insertions(+), 48 deletions(-)
 create mode 100644 tools/include/nolibc/inttypes.h
 create mode 100644 tools/include/nolibc/sys/select.h

-- 
2.17.5

[PATCH 1/4] tools/nolibc: fix misleading help message regarding installation path

[Willy Tarreau]

The help message says the headers are going to be installed into
tools/include/nolibc but this is only the default if $OUTPUT is not set,
so better clarify this (the current value of $OUTPUT is already shown).

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index c335ce0bd195c..f91d29705733a 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -80,7 +80,7 @@ help:
 	@echo "Supported targets under nolibc:"
 	@echo "  all                 call \"headers\""
 	@echo "  clean               clean the sysroot"
-	@echo "  headers             prepare a sysroot in tools/include/nolibc/sysroot"
+	@echo "  headers             prepare a sysroot in \$${OUTPUT}sysroot"
 	@echo "  headers_standalone  like \"headers\", and also install kernel headers"
 	@echo "  help                this help"
 	@echo ""
-- 
2.17.5

[PATCH 2/4] tools/nolibc: add the more portable inttypes.h

[Willy Tarreau]

It's often recommended to only use inttypes.h instead of stdint.h for
portability reasons since the former is always present when the latter
is present, but not conversely, and the former includes the latter. Due
to this some simple programs fail to build when including inttypes.h.
Let's add one that simply includes stdint.h to better support these
programs.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/Makefile   | 1 +
 tools/include/nolibc/inttypes.h | 8 ++++++++
 2 files changed, 9 insertions(+)
 create mode 100644 tools/include/nolibc/inttypes.h

diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index f91d29705733a..725cf49516185 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -34,6 +34,7 @@ all_files := \
 		errno.h \
 		fcntl.h \
 		getopt.h \
+		inttypes.h \
 		limits.h \
 		math.h \
 		nolibc.h \
diff --git a/tools/include/nolibc/inttypes.h b/tools/include/nolibc/inttypes.h
new file mode 100644
index 0000000000000..1088034ef7cca
--- /dev/null
+++ b/tools/include/nolibc/inttypes.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+
+#ifndef _NOLIBC_INTTYPES_H
+#define _NOLIBC_INTTYPES_H
+
+#include "stdint.h"
+
+#endif /* _NOLIBC_INTTYPES_H */
-- 
2.17.5

[PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

Modern programs tend to include sys/select.h to get FD_SET() and
FD_CLR() definitions as well as struct fd_set, but in our case it
didn't exist. Let's move these definitions from types.h to sys/select.h
to help port existing programs.

Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 tools/include/nolibc/Makefile     |  1 +
 tools/include/nolibc/sys/select.h | 56 +++++++++++++++++++++++++++++++
 tools/include/nolibc/types.h      | 48 +-------------------------
 3 files changed, 58 insertions(+), 47 deletions(-)
 create mode 100644 tools/include/nolibc/sys/select.h

diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index 725cf49516185..823bfa08c6756 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -58,6 +58,7 @@ all_files := \
 		sys/random.h \
 		sys/reboot.h \
 		sys/resource.h \
+		sys/select.h \
 		sys/stat.h \
 		sys/syscall.h \
 		sys/sysmacros.h \
diff --git a/tools/include/nolibc/sys/select.h b/tools/include/nolibc/sys/select.h
new file mode 100644
index 0000000000000..a7481592c76f4
--- /dev/null
+++ b/tools/include/nolibc/sys/select.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+
+/* make sure to include all global symbols */
+#include "../nolibc.h"
+
+#ifndef _NOLIBC_SYS_SELECT_H
+#define _NOLIBC_SYS_SELECT_H
+
+/* commonly an fd_set represents 256 FDs */
+#ifndef FD_SETSIZE
+#define FD_SETSIZE     256
+#endif
+
+#define FD_SETIDXMASK (8 * sizeof(unsigned long))
+#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
+
+/* for select() */
+typedef struct {
+	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
+} fd_set;
+
+#define FD_CLR(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] &=		\
+				~(1U << (__fd & FD_SETBITMASK));	\
+	} while (0)
+
+#define FD_SET(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] |=		\
+				1 << (__fd & FD_SETBITMASK);		\
+	} while (0)
+
+#define FD_ISSET(fd, set) ({						\
+			fd_set *__set = (set);				\
+			int __fd = (fd);				\
+		int __r = 0;						\
+		if (__fd >= 0)						\
+			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
+1U << (__fd & FD_SETBITMASK));						\
+		__r;							\
+	})
+
+#define FD_ZERO(set) do {						\
+		fd_set *__set = (set);					\
+		int __idx;						\
+		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
+		for (__idx = 0; __idx < __size; __idx++)		\
+			__set->fds[__idx] = 0;				\
+	} while (0)
+
+#endif /* _NOLIBC_SYS_SELECT_H */
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 16c6e9ec9451f..0b51ede4e0a9c 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -10,6 +10,7 @@
 #ifndef _NOLIBC_TYPES_H
 #define _NOLIBC_TYPES_H
 
+#include "sys/select.h"
 #include "std.h"
 #include <linux/mman.h>
 #include <linux/stat.h>
@@ -70,11 +71,6 @@
 #define DT_LNK         0xa
 #define DT_SOCK        0xc
 
-/* commonly an fd_set represents 256 FDs */
-#ifndef FD_SETSIZE
-#define FD_SETSIZE     256
-#endif
-
 /* PATH_MAX and MAXPATHLEN are often used and found with plenty of different
  * values.
  */
@@ -115,48 +111,6 @@
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
 
-#define FD_SETIDXMASK (8 * sizeof(unsigned long))
-#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
-
-/* for select() */
-typedef struct {
-	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
-} fd_set;
-
-#define FD_CLR(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] &=		\
-				~(1U << (__fd & FD_SETBITMASK));	\
-	} while (0)
-
-#define FD_SET(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] |=		\
-				1 << (__fd & FD_SETBITMASK);		\
-	} while (0)
-
-#define FD_ISSET(fd, set) ({						\
-			fd_set *__set = (set);				\
-			int __fd = (fd);				\
-		int __r = 0;						\
-		if (__fd >= 0)						\
-			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
-1U << (__fd & FD_SETBITMASK));						\
-		__r;							\
-	})
-
-#define FD_ZERO(set) do {						\
-		fd_set *__set = (set);					\
-		int __idx;						\
-		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
-		for (__idx = 0; __idx < __size; __idx++)		\
-			__set->fds[__idx] = 0;				\
-	} while (0)
-
 /* for getdents64() */
 struct linux_dirent64 {
 	uint64_t       d_ino;
-- 
2.17.5

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> Modern programs tend to include sys/select.h to get FD_SET() and
> FD_CLR() definitions as well as struct fd_set, but in our case it
> didn't exist. Let's move these definitions from types.h to sys/select.h
> to help port existing programs.
> 
> Signed-off-by: Willy Tarreau <w@1wt.eu>

<snip>

> diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> index 16c6e9ec9451f..0b51ede4e0a9c 100644
> --- a/tools/include/nolibc/types.h
> +++ b/tools/include/nolibc/types.h
> @@ -10,6 +10,7 @@
>  #ifndef _NOLIBC_TYPES_H
>  #define _NOLIBC_TYPES_H
>  
> +#include "sys/select.h"

Is this really necessary?

>  #include "std.h"
>  #include <linux/mman.h>
>  #include <linux/stat.h>
> @@ -70,11 +71,6 @@
>  #define DT_LNK         0xa
>  #define DT_SOCK        0xc

<snip>

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

Hi Thomas,

On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > Modern programs tend to include sys/select.h to get FD_SET() and
> > FD_CLR() definitions as well as struct fd_set, but in our case it
> > didn't exist. Let's move these definitions from types.h to sys/select.h
> > to help port existing programs.
> > 
> > Signed-off-by: Willy Tarreau <w@1wt.eu>
> 
> <snip>
> 
> > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > --- a/tools/include/nolibc/types.h
> > +++ b/tools/include/nolibc/types.h
> > @@ -10,6 +10,7 @@
> >  #ifndef _NOLIBC_TYPES_H
> >  #define _NOLIBC_TYPES_H
> >  
> > +#include "sys/select.h"
> 
> Is this really necessary?

Not sure what you mean. Do you mean that you would have preferred it
to be included from nolibc.h instead (which I'm equally fine with) or
that you'd prefer to have an empty sys/select.h ?

Thanks,
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-22 09:19:58+0200, Willy Tarreau wrote:
> Hi Thomas,
> 
> On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> > On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > > Modern programs tend to include sys/select.h to get FD_SET() and
> > > FD_CLR() definitions as well as struct fd_set, but in our case it
> > > didn't exist. Let's move these definitions from types.h to sys/select.h
> > > to help port existing programs.
> > > 
> > > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > 
> > <snip>
> > 
> > > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > > --- a/tools/include/nolibc/types.h
> > > +++ b/tools/include/nolibc/types.h
> > > @@ -10,6 +10,7 @@
> > >  #ifndef _NOLIBC_TYPES_H
> > >  #define _NOLIBC_TYPES_H
> > >  
> > > +#include "sys/select.h"
> > 
> > Is this really necessary?
> 
> Not sure what you mean. Do you mean that you would have preferred it
> to be included from nolibc.h instead (which I'm equally fine with) or
> that you'd prefer to have an empty sys/select.h ?

The former.

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 22, 2025 at 09:58:52PM +0200, Thomas Weißschuh wrote:
> On 2025-06-22 09:19:58+0200, Willy Tarreau wrote:
> > Hi Thomas,
> > 
> > On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> > > On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > > > Modern programs tend to include sys/select.h to get FD_SET() and
> > > > FD_CLR() definitions as well as struct fd_set, but in our case it
> > > > didn't exist. Let's move these definitions from types.h to sys/select.h
> > > > to help port existing programs.
> > > > 
> > > > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > > 
> > > <snip>
> > > 
> > > > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > > > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > > > --- a/tools/include/nolibc/types.h
> > > > +++ b/tools/include/nolibc/types.h
> > > > @@ -10,6 +10,7 @@
> > > >  #ifndef _NOLIBC_TYPES_H
> > > >  #define _NOLIBC_TYPES_H
> > > >  
> > > > +#include "sys/select.h"
> > > 
> > > Is this really necessary?
> > 
> > Not sure what you mean. Do you mean that you would have preferred it
> > to be included from nolibc.h instead (which I'm equally fine with) or
> > that you'd prefer to have an empty sys/select.h ?
> 
> The former.

OK thanks, you're right, that's more consistent with the rest,
I'll do that and push it.

Thanks!
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> On Sun, Jun 22, 2025 at 09:58:52PM +0200, Thomas Weißschuh wrote:
> > On 2025-06-22 09:19:58+0200, Willy Tarreau wrote:
> > > Hi Thomas,
> > > 
> > > On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> > > > On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > > > > Modern programs tend to include sys/select.h to get FD_SET() and
> > > > > FD_CLR() definitions as well as struct fd_set, but in our case it
> > > > > didn't exist. Let's move these definitions from types.h to sys/select.h
> > > > > to help port existing programs.
> > > > > 
> > > > > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > > > 
> > > > <snip>
> > > > 
> > > > > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > > > > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > > > > --- a/tools/include/nolibc/types.h
> > > > > +++ b/tools/include/nolibc/types.h
> > > > > @@ -10,6 +10,7 @@
> > > > >  #ifndef _NOLIBC_TYPES_H
> > > > >  #define _NOLIBC_TYPES_H
> > > > >  
> > > > > +#include "sys/select.h"
> > > > 
> > > > Is this really necessary?
> > > 
> > > Not sure what you mean. Do you mean that you would have preferred it
> > > to be included from nolibc.h instead (which I'm equally fine with) or
> > > that you'd prefer to have an empty sys/select.h ?
> > 
> > The former.
> 
> OK thanks, you're right, that's more consistent with the rest,
> I'll do that and push it.

Trying it has reopened the circular dependencies can of worms :-(
It's the same problem as usual that we've worked around till now
by placing some types in types.h, except that this time fd_set is
defined based on the macros FD_* that I moved to sys/select.h.

I'm giving up on this one for now as I don't want us to revisit
that painful dependencies sequence. In theory it should be as simple
as guarding types and function definitions independently, but in
reality it's never as rocket science as it can also pop up in macros
and rare typedefs.

Instead I'll just provide a stub for sys/select.h just like for
inttypes so that user code compiles without changing existing files.

Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> > On Sun, Jun 22, 2025 at 09:58:52PM +0200, Thomas Weißschuh wrote:
> > > On 2025-06-22 09:19:58+0200, Willy Tarreau wrote:
> > > > Hi Thomas,
> > > > 
> > > > On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> > > > > On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > > > > > Modern programs tend to include sys/select.h to get FD_SET() and
> > > > > > FD_CLR() definitions as well as struct fd_set, but in our case it
> > > > > > didn't exist. Let's move these definitions from types.h to sys/select.h
> > > > > > to help port existing programs.
> > > > > > 
> > > > > > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > > > > 
> > > > > <snip>
> > > > > 
> > > > > > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > > > > > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > > > > > --- a/tools/include/nolibc/types.h
> > > > > > +++ b/tools/include/nolibc/types.h
> > > > > > @@ -10,6 +10,7 @@
> > > > > >  #ifndef _NOLIBC_TYPES_H
> > > > > >  #define _NOLIBC_TYPES_H
> > > > > >  
> > > > > > +#include "sys/select.h"
> > > > > 
> > > > > Is this really necessary?
> > > > 
> > > > Not sure what you mean. Do you mean that you would have preferred it
> > > > to be included from nolibc.h instead (which I'm equally fine with) or
> > > > that you'd prefer to have an empty sys/select.h ?
> > > 
> > > The former.
> > 
> > OK thanks, you're right, that's more consistent with the rest,
> > I'll do that and push it.
> 
> Trying it has reopened the circular dependencies can of worms :-(
> It's the same problem as usual that we've worked around till now
> by placing some types in types.h, except that this time fd_set is
> defined based on the macros FD_* that I moved to sys/select.h.

Can't fd_set also move to sys/select.h? This is how I read fd_set(3).
Even if it is not standards compliant, for nolibc it won't matter as in
the end everything is available anyaways.

> I'm giving up on this one for now as I don't want us to revisit
> that painful dependencies sequence. In theory it should be as simple
> as guarding types and function definitions independently, but in
> reality it's never as rocket science as it can also pop up in macros
> and rare typedefs.

Also the headers are always included in the order written down in
nolibc.h, so sys/select could be above types.h there.

> Instead I'll just provide a stub for sys/select.h just like for
> inttypes so that user code compiles without changing existing files.

That works for me, too.

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 10:53:34AM +0200, Thomas Weißschuh wrote:
> On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> > On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> > > On Sun, Jun 22, 2025 at 09:58:52PM +0200, Thomas Weißschuh wrote:
> > > > On 2025-06-22 09:19:58+0200, Willy Tarreau wrote:
> > > > > Hi Thomas,
> > > > > 
> > > > > On Sat, Jun 21, 2025 at 10:21:47AM +0200, Thomas Weißschuh wrote:
> > > > > > On 2025-06-20 12:02:50+0200, Willy Tarreau wrote:
> > > > > > > Modern programs tend to include sys/select.h to get FD_SET() and
> > > > > > > FD_CLR() definitions as well as struct fd_set, but in our case it
> > > > > > > didn't exist. Let's move these definitions from types.h to sys/select.h
> > > > > > > to help port existing programs.
> > > > > > > 
> > > > > > > Signed-off-by: Willy Tarreau <w@1wt.eu>
> > > > > > 
> > > > > > <snip>
> > > > > > 
> > > > > > > diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
> > > > > > > index 16c6e9ec9451f..0b51ede4e0a9c 100644
> > > > > > > --- a/tools/include/nolibc/types.h
> > > > > > > +++ b/tools/include/nolibc/types.h
> > > > > > > @@ -10,6 +10,7 @@
> > > > > > >  #ifndef _NOLIBC_TYPES_H
> > > > > > >  #define _NOLIBC_TYPES_H
> > > > > > >  
> > > > > > > +#include "sys/select.h"
> > > > > > 
> > > > > > Is this really necessary?
> > > > > 
> > > > > Not sure what you mean. Do you mean that you would have preferred it
> > > > > to be included from nolibc.h instead (which I'm equally fine with) or
> > > > > that you'd prefer to have an empty sys/select.h ?
> > > > 
> > > > The former.
> > > 
> > > OK thanks, you're right, that's more consistent with the rest,
> > > I'll do that and push it.
> > 
> > Trying it has reopened the circular dependencies can of worms :-(
> > It's the same problem as usual that we've worked around till now
> > by placing some types in types.h, except that this time fd_set is
> > defined based on the macros FD_* that I moved to sys/select.h.
> 
> Can't fd_set also move to sys/select.h? This is how I read fd_set(3).

That was what I did and precisely what was causing the problem. We
have sys.h defining select() with fd_set in it with sys/select not yet
being included. I moved sys.h after all sys/* and it broke something
else instead.

> Even if it is not standards compliant, for nolibc it won't matter as in
> the end everything is available anyaways.

Yeah I totally agree with you on that!

> > I'm giving up on this one for now as I don't want us to revisit
> > that painful dependencies sequence. In theory it should be as simple
> > as guarding types and function definitions independently, but in
> > reality it's never as rocket science as it can also pop up in macros
> > and rare typedefs.
> 
> Also the headers are always included in the order written down in
> nolibc.h, so sys/select could be above types.h there.

In an ideal world we'd have all sys/* *before* anything else. But I
do remember that we faced a few issues doing this. This can be refined
though. I'm just careful because we have not been annoyed in a while,
so I suspect we're close to something good and it's easy to cause more
breakage than fixes by touching all of this.

> > Instead I'll just provide a stub for sys/select.h just like for
> > inttypes so that user code compiles without changing existing files.
> 
> That works for me, too.

That's what I did in the end. We're just addressing annoying build
issues and not providing new stuff that deserves a full reorganization!

Cheers,
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-29 11:25:52+0200, Willy Tarreau wrote:
> On Sun, Jun 29, 2025 at 10:53:34AM +0200, Thomas Weißschuh wrote:
> > On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> > > On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:

<snip>

> > > Trying it has reopened the circular dependencies can of worms :-(
> > > It's the same problem as usual that we've worked around till now
> > > by placing some types in types.h, except that this time fd_set is
> > > defined based on the macros FD_* that I moved to sys/select.h.
> > 
> > Can't fd_set also move to sys/select.h? This is how I read fd_set(3).
> 
> That was what I did and precisely what was causing the problem. We
> have sys.h defining select() with fd_set in it with sys/select not yet
> being included. I moved sys.h after all sys/* and it broke something
> else instead.

Ah. Then move select() also into sys/select.h; where it belongs. :-)

<snip>

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 11:37:06AM +0200, Thomas Weißschuh wrote:
> On 2025-06-29 11:25:52+0200, Willy Tarreau wrote:
> > On Sun, Jun 29, 2025 at 10:53:34AM +0200, Thomas Weißschuh wrote:
> > > On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> > > > On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> 
> <snip>
> 
> > > > Trying it has reopened the circular dependencies can of worms :-(
> > > > It's the same problem as usual that we've worked around till now
> > > > by placing some types in types.h, except that this time fd_set is
> > > > defined based on the macros FD_* that I moved to sys/select.h.
> > > 
> > > Can't fd_set also move to sys/select.h? This is how I read fd_set(3).
> > 
> > That was what I did and precisely what was causing the problem. We
> > have sys.h defining select() with fd_set in it with sys/select not yet
> > being included. I moved sys.h after all sys/* and it broke something
> > else instead.
> 
> Ah. Then move select() also into sys/select.h; where it belongs. :-)

For an unknown reason I thought we avoided to move the syscall definitions
there and only used sys/*, but I was apparently confused as we have exactly
that in prctl or wait. I can give that one a try again.

Thanks,
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 11:40:48AM +0200, Willy Tarreau wrote:
> On Sun, Jun 29, 2025 at 11:37:06AM +0200, Thomas Weißschuh wrote:
> > On 2025-06-29 11:25:52+0200, Willy Tarreau wrote:
> > > On Sun, Jun 29, 2025 at 10:53:34AM +0200, Thomas Weißschuh wrote:
> > > > On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> > > > > On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> > 
> > <snip>
> > 
> > > > > Trying it has reopened the circular dependencies can of worms :-(
> > > > > It's the same problem as usual that we've worked around till now
> > > > > by placing some types in types.h, except that this time fd_set is
> > > > > defined based on the macros FD_* that I moved to sys/select.h.
> > > > 
> > > > Can't fd_set also move to sys/select.h? This is how I read fd_set(3).
> > > 
> > > That was what I did and precisely what was causing the problem. We
> > > have sys.h defining select() with fd_set in it with sys/select not yet
> > > being included. I moved sys.h after all sys/* and it broke something
> > > else instead.
> > 
> > Ah. Then move select() also into sys/select.h; where it belongs. :-)
> 
> For an unknown reason I thought we avoided to move the syscall definitions
> there and only used sys/*, but I was apparently confused as we have exactly
> that in prctl or wait. I can give that one a try again.

So after one more hour on it, I'm admitting abandonning the battle.
Adding the necessary includes there is causing "declared inside parameter"
failures in random other totally unrelated stuff (e.g. in sys_getdents64()
or msleep()). We'll have to really clear that circular includes mess again
in a near future. For now I'll stay on the stub which works fine without
affecting the rest.

Cheers,
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-29 17:10:06+0200, Willy Tarreau wrote:
> On Sun, Jun 29, 2025 at 11:40:48AM +0200, Willy Tarreau wrote:
> > On Sun, Jun 29, 2025 at 11:37:06AM +0200, Thomas Weißschuh wrote:
> > > On 2025-06-29 11:25:52+0200, Willy Tarreau wrote:
> > > > On Sun, Jun 29, 2025 at 10:53:34AM +0200, Thomas Weißschuh wrote:
> > > > > On 2025-06-29 10:46:28+0200, Willy Tarreau wrote:
> > > > > > On Mon, Jun 23, 2025 at 04:56:18AM +0200, Willy Tarreau wrote:
> > > 
> > > <snip>
> > > 
> > > > > > Trying it has reopened the circular dependencies can of worms :-(
> > > > > > It's the same problem as usual that we've worked around till now
> > > > > > by placing some types in types.h, except that this time fd_set is
> > > > > > defined based on the macros FD_* that I moved to sys/select.h.
> > > > > 
> > > > > Can't fd_set also move to sys/select.h? This is how I read fd_set(3).
> > > > 
> > > > That was what I did and precisely what was causing the problem. We
> > > > have sys.h defining select() with fd_set in it with sys/select not yet
> > > > being included. I moved sys.h after all sys/* and it broke something
> > > > else instead.
> > > 
> > > Ah. Then move select() also into sys/select.h; where it belongs. :-)
> > 
> > For an unknown reason I thought we avoided to move the syscall definitions
> > there and only used sys/*, but I was apparently confused as we have exactly
> > that in prctl or wait. I can give that one a try again.
> 
> So after one more hour on it, I'm admitting abandonning the battle.
> Adding the necessary includes there is causing "declared inside parameter"
> failures in random other totally unrelated stuff (e.g. in sys_getdents64()
> or msleep()). We'll have to really clear that circular includes mess again
> in a near future. For now I'll stay on the stub which works fine without
> affecting the rest.

I saw the same issue, but only because of the changes to types.h.
And these should not be necessary in the first place.

The below works nicely for me:

diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index 3fcee9fe4ece..125dbb2f1388 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -56,6 +56,7 @@ all_files := \
 		sys/random.h \
 		sys/reboot.h \
 		sys/resource.h \
+		sys/select.h \
 		sys/stat.h \
 		sys/syscall.h \
 		sys/sysmacros.h \
diff --git a/tools/include/nolibc/nolibc.h b/tools/include/nolibc/nolibc.h
index c199ade200c2..6dc2f2a6cbde 100644
--- a/tools/include/nolibc/nolibc.h
+++ b/tools/include/nolibc/nolibc.h
@@ -104,6 +104,7 @@
 #include "sys/random.h"
 #include "sys/reboot.h"
 #include "sys/resource.h"
+#include "sys/select.h"
 #include "sys/stat.h"
 #include "sys/syscall.h"
 #include "sys/sysmacros.h"
diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 9556c69a6ae1..ae4b0970b570 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -745,53 +745,6 @@ int sched_yield(void)
 }
 
 
-/*
- * int select(int nfds, fd_set *read_fds, fd_set *write_fds,
- *            fd_set *except_fds, struct timeval *timeout);
- */
-
-static __attribute__((unused))
-int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeval *timeout)
-{
-#if defined(__ARCH_WANT_SYS_OLD_SELECT) && !defined(__NR__newselect)
-	struct sel_arg_struct {
-		unsigned long n;
-		fd_set *r, *w, *e;
-		struct timeval *t;
-	} arg = { .n = nfds, .r = rfds, .w = wfds, .e = efds, .t = timeout };
-	return my_syscall1(__NR_select, &arg);
-#elif defined(__NR__newselect)
-	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
-#elif defined(__NR_select)
-	return my_syscall5(__NR_select, nfds, rfds, wfds, efds, timeout);
-#elif defined(__NR_pselect6)
-	struct timespec t;
-
-	if (timeout) {
-		t.tv_sec  = timeout->tv_sec;
-		t.tv_nsec = timeout->tv_usec * 1000;
-	}
-	return my_syscall6(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
-#elif defined(__NR_pselect6_time64)
-	struct __kernel_timespec t;
-
-	if (timeout) {
-		t.tv_sec  = timeout->tv_sec;
-		t.tv_nsec = timeout->tv_usec * 1000;
-	}
-	return my_syscall6(__NR_pselect6_time64, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
-#else
-	return __nolibc_enosys(__func__, nfds, rfds, wfds, efds, timeout);
-#endif
-}
-
-static __attribute__((unused))
-int select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeval *timeout)
-{
-	return __sysret(sys_select(nfds, rfds, wfds, efds, timeout));
-}
-
-
 /*
  * int setpgid(pid_t pid, pid_t pgid);
  */
diff --git a/tools/include/nolibc/sys/select.h b/tools/include/nolibc/sys/select.h
new file mode 100644
index 000000000000..74d0e55e3157
--- /dev/null
+++ b/tools/include/nolibc/sys/select.h
@@ -0,0 +1,109 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+
+/* make sure to include all global symbols */
+#include "../nolibc.h"
+
+#ifndef _NOLIBC_SYS_SELECT_H
+#define _NOLIBC_SYS_SELECT_H
+
+#include <linux/time.h>
+#include <linux/unistd.h>
+
+#include "../sys.h"
+
+/* commonly an fd_set represents 256 FDs */
+#ifndef FD_SETSIZE
+#define FD_SETSIZE     256
+#endif
+
+#define FD_SETIDXMASK (8 * sizeof(unsigned long))
+#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
+
+/* for select() */
+typedef struct {
+	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
+} fd_set;
+
+#define FD_CLR(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] &=		\
+				~(1U << (__fd & FD_SETBITMASK));	\
+	} while (0)
+
+#define FD_SET(fd, set) do {						\
+		fd_set *__set = (set);					\
+		int __fd = (fd);					\
+		if (__fd >= 0)						\
+			__set->fds[__fd / FD_SETIDXMASK] |=		\
+				1 << (__fd & FD_SETBITMASK);		\
+	} while (0)
+
+#define FD_ISSET(fd, set) ({						\
+			fd_set *__set = (set);				\
+			int __fd = (fd);				\
+		int __r = 0;						\
+		if (__fd >= 0)						\
+			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
+1U << (__fd & FD_SETBITMASK));						\
+		__r;							\
+	})
+
+#define FD_ZERO(set) do {						\
+		fd_set *__set = (set);					\
+		int __idx;						\
+		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
+		for (__idx = 0; __idx < __size; __idx++)		\
+			__set->fds[__idx] = 0;				\
+	} while (0)
+
+
+/*
+ * int select(int nfds, fd_set *read_fds, fd_set *write_fds,
+ *            fd_set *except_fds, struct timeval *timeout);
+ */
+
+static __attribute__((unused))
+int sys_select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeval *timeout)
+{
+#if defined(__ARCH_WANT_SYS_OLD_SELECT) && !defined(__NR__newselect)
+	struct sel_arg_struct {
+		unsigned long n;
+		fd_set *r, *w, *e;
+		struct timeval *t;
+	} arg = { .n = nfds, .r = rfds, .w = wfds, .e = efds, .t = timeout };
+	return my_syscall1(__NR_select, &arg);
+#elif defined(__NR__newselect)
+	return my_syscall5(__NR__newselect, nfds, rfds, wfds, efds, timeout);
+#elif defined(__NR_select)
+	return my_syscall5(__NR_select, nfds, rfds, wfds, efds, timeout);
+#elif defined(__NR_pselect6)
+	struct timespec t;
+
+	if (timeout) {
+		t.tv_sec  = timeout->tv_sec;
+		t.tv_nsec = timeout->tv_usec * 1000;
+	}
+	return my_syscall6(__NR_pselect6, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
+#elif defined(__NR_pselect6_time64)
+	struct __kernel_timespec t;
+
+	if (timeout) {
+		t.tv_sec  = timeout->tv_sec;
+		t.tv_nsec = timeout->tv_usec * 1000;
+	}
+	return my_syscall6(__NR_pselect6_time64, nfds, rfds, wfds, efds, timeout ? &t : NULL, NULL);
+#else
+	return __nolibc_enosys(__func__, nfds, rfds, wfds, efds, timeout);
+#endif
+}
+
+static __attribute__((unused))
+int select(int nfds, fd_set *rfds, fd_set *wfds, fd_set *efds, struct timeval *timeout)
+{
+	return __sysret(sys_select(nfds, rfds, wfds, efds, timeout));
+}
+
+
+#endif /* _NOLIBC_SYS_SELECT_H */
diff --git a/tools/include/nolibc/types.h b/tools/include/nolibc/types.h
index 16c6e9ec9451..470a5f77bc0f 100644
--- a/tools/include/nolibc/types.h
+++ b/tools/include/nolibc/types.h
@@ -70,11 +70,6 @@
 #define DT_LNK         0xa
 #define DT_SOCK        0xc
 
-/* commonly an fd_set represents 256 FDs */
-#ifndef FD_SETSIZE
-#define FD_SETSIZE     256
-#endif
-
 /* PATH_MAX and MAXPATHLEN are often used and found with plenty of different
  * values.
  */
@@ -115,48 +110,6 @@
 #define EXIT_SUCCESS 0
 #define EXIT_FAILURE 1
 
-#define FD_SETIDXMASK (8 * sizeof(unsigned long))
-#define FD_SETBITMASK (8 * sizeof(unsigned long)-1)
-
-/* for select() */
-typedef struct {
-	unsigned long fds[(FD_SETSIZE + FD_SETBITMASK) / FD_SETIDXMASK];
-} fd_set;
-
-#define FD_CLR(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] &=		\
-				~(1U << (__fd & FD_SETBITMASK));	\
-	} while (0)
-
-#define FD_SET(fd, set) do {						\
-		fd_set *__set = (set);					\
-		int __fd = (fd);					\
-		if (__fd >= 0)						\
-			__set->fds[__fd / FD_SETIDXMASK] |=		\
-				1 << (__fd & FD_SETBITMASK);		\
-	} while (0)
-
-#define FD_ISSET(fd, set) ({						\
-			fd_set *__set = (set);				\
-			int __fd = (fd);				\
-		int __r = 0;						\
-		if (__fd >= 0)						\
-			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
-1U << (__fd & FD_SETBITMASK));						\
-		__r;							\
-	})
-
-#define FD_ZERO(set) do {						\
-		fd_set *__set = (set);					\
-		int __idx;						\
-		int __size = (FD_SETSIZE+FD_SETBITMASK) / FD_SETIDXMASK;\
-		for (__idx = 0; __idx < __size; __idx++)		\
-			__set->fds[__idx] = 0;				\
-	} while (0)
-
 /* for getdents64() */
 struct linux_dirent64 {
 	uint64_t       d_ino;

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 05:54:02PM +0200, Thomas Weißschuh wrote:
> I saw the same issue, but only because of the changes to types.h.
> And these should not be necessary in the first place.

You mean fd_set definition ? It's solely a matter of includes ordering
in fact, since it depends on FD_SETSIZE.

> The below works nicely for me:

OK. Do you prefer that we take that one or just a stub ? I'm personally
fine with both.

Thanks!
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-29 18:08:23+0200, Willy Tarreau wrote:
> On Sun, Jun 29, 2025 at 05:54:02PM +0200, Thomas Weißschuh wrote:
> > I saw the same issue, but only because of the changes to types.h.
> > And these should not be necessary in the first place.
> 
> You mean fd_set definition ? It's solely a matter of includes ordering
> in fact, since it depends on FD_SETSIZE.

No, I mean the '#include "sys/select.h"' in "types.h".
That breaks the dependency order, as it pulls in all kinds of other
stuff into the beginning of "types.h" which themselves depend on
definitions of "types.h".

> > The below works nicely for me:
> 
> OK. Do you prefer that we take that one or just a stub ? I'm personally
> fine with both.

I prefer moving the code.

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 06:14:01PM +0200, Thomas Weißschuh wrote:
> On 2025-06-29 18:08:23+0200, Willy Tarreau wrote:
> > On Sun, Jun 29, 2025 at 05:54:02PM +0200, Thomas Weißschuh wrote:
> > > I saw the same issue, but only because of the changes to types.h.
> > > And these should not be necessary in the first place.
> > 
> > You mean fd_set definition ? It's solely a matter of includes ordering
> > in fact, since it depends on FD_SETSIZE.
> 
> No, I mean the '#include "sys/select.h"' in "types.h".

I had already dropped it as well.

> That breaks the dependency order, as it pulls in all kinds of other
> stuff into the beginning of "types.h" which themselves depend on
> definitions of "types.h".

It was not just this. I'm pretty sure that what unbroke it for you is
keeping fd_set in types.h.

> > > The below works nicely for me:
> > 
> > OK. Do you prefer that we take that one or just a stub ? I'm personally
> > fine with both.
> 
> I prefer moving the code.

OK. Do you want me to merge your patch or will you take care of it ?

Thanks,
Willy

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Thomas Weißschuh]

On 2025-06-29 18:19:08+0200, Willy Tarreau wrote:
> On Sun, Jun 29, 2025 at 06:14:01PM +0200, Thomas Weißschuh wrote:
> > On 2025-06-29 18:08:23+0200, Willy Tarreau wrote:
> > > On Sun, Jun 29, 2025 at 05:54:02PM +0200, Thomas Weißschuh wrote:
> > > > I saw the same issue, but only because of the changes to types.h.
> > > > And these should not be necessary in the first place.
> > > 
> > > You mean fd_set definition ? It's solely a matter of includes ordering
> > > in fact, since it depends on FD_SETSIZE.
> > 
> > No, I mean the '#include "sys/select.h"' in "types.h".
> 
> I had already dropped it as well.

Hm, then I'm not sure where it commes from.

> > That breaks the dependency order, as it pulls in all kinds of other
> > stuff into the beginning of "types.h" which themselves depend on
> > definitions of "types.h".
> 
> It was not just this. I'm pretty sure that what unbroke it for you is
> keeping fd_set in types.h.

But I *did* move fd_set to sys/select.h.

> > > > The below works nicely for me:
> > > 
> > > OK. Do you prefer that we take that one or just a stub ? I'm personally
> > > fine with both.
> > 
> > I prefer moving the code.
> 
> OK. Do you want me to merge your patch or will you take care of it ?

As you'll resubmit your series anyways, please pick up my proposal.

Re: [PATCH 3/4] tools/nolibc: move FD_* definitions to sys/select.h

[Willy Tarreau]

On Sun, Jun 29, 2025 at 06:27:27PM +0200, Thomas Weißschuh wrote:
> On 2025-06-29 18:19:08+0200, Willy Tarreau wrote:
> > On Sun, Jun 29, 2025 at 06:14:01PM +0200, Thomas Weißschuh wrote:
> > > On 2025-06-29 18:08:23+0200, Willy Tarreau wrote:
> > > > On Sun, Jun 29, 2025 at 05:54:02PM +0200, Thomas Weißschuh wrote:
> > > > > I saw the same issue, but only because of the changes to types.h.
> > > > > And these should not be necessary in the first place.
> > > > 
> > > > You mean fd_set definition ? It's solely a matter of includes ordering
> > > > in fact, since it depends on FD_SETSIZE.
> > > 
> > > No, I mean the '#include "sys/select.h"' in "types.h".
> > 
> > I had already dropped it as well.
> 
> Hm, then I'm not sure where it commes from.

I noticed that you included linux/unistd.h from sys/select.h, which I
didn't have, maybe it plays a role as well.

> > > That breaks the dependency order, as it pulls in all kinds of other
> > > stuff into the beginning of "types.h" which themselves depend on
> > > definitions of "types.h".
> > 
> > It was not just this. I'm pretty sure that what unbroke it for you is
> > keeping fd_set in types.h.
> 
> But I *did* move fd_set to sys/select.h.

Sorry then, I just didn't notice it while trying to comapre the
differences.

> > > > > The below works nicely for me:
> > > > 
> > > > OK. Do you prefer that we take that one or just a stub ? I'm personally
> > > > fine with both.
> > > 
> > > I prefer moving the code.
> > 
> > OK. Do you want me to merge your patch or will you take care of it ?
> 
> As you'll resubmit your series anyways, please pick up my proposal.

OK will do that. Thanks.
willy

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

[Willy Tarreau]

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
+static __attribute__((unused))
+void *memchr(const void *s, int c, size_t len)
+{
+	char *p = (char *)s;
+
+	while (len--) {
+		if (*p == (char)c)
+			return p;
+		p++;
+	}
+	return NULL;
+}
+#endif /* #ifndef NOLIBC_ARCH_HAS_MEMCHR */
+
 static __attribute__((unused))
 char *strchr(const char *s, int c)
 {
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index dbe13000fb1ac..d832566265296 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1524,6 +1524,8 @@ int run_stdlib(int min, int max)
 		CASE_TEST(abs);                     EXPECT_EQ(1, abs(-10), 10); break;
 		CASE_TEST(abs_noop);                EXPECT_EQ(1, abs(10), 10); break;
 		CASE_TEST(difftime);                EXPECT_ZR(1, test_difftime()); break;
+		CASE_TEST(memchr_foobar6_o);        EXPECT_STREQ(1, memchr("foobar", 'o', 6), "oobar"); break;
+		CASE_TEST(memchr_foobar3_b);        EXPECT_STRZR(1, memchr("foobar", 'b', 3)); break;
 
 		case __LINE__:
 			return ret; /* must be last */
-- 
2.17.5

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

[Thomas Weißschuh]

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?

> +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".
Also, can we keep the const?

> +
> +	while (len--) {
> +		if (*p == (char)c)
> +			return p;
> +		p++;
> +	}
> +	return NULL;
> +}
> +#endif /* #ifndef NOLIBC_ARCH_HAS_MEMCHR */
> +
>  static __attribute__((unused))
>  char *strchr(const char *s, int c)
>  {

<snip>

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

[Willy Tarreau]

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

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

[Thomas Weißschuh]

On 2025-06-21 10:42:34+0200, Willy Tarreau wrote:
> 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).

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.

> > > +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.

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

[Willy Tarreau]

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