On Tue, Mar 04, 2025 at 08:54:29AM +0100, Willy Tarreau wrote:
On Tue, Mar 04, 2025 at 08:10:54AM +0100, Thomas Weißschuh wrote:
diff --git a/tools/include/nolibc/getopt.h b/tools/include/nolibc/getopt.h new file mode 100644 index 0000000000000000000000000000000000000000..35aee582681b79e21bce8ddbf634ae9dfdef8f1d --- /dev/null +++ b/tools/include/nolibc/getopt.h @@ -0,0 +1,105 @@ +/* SPDX-License-Identifier: LGPL-2.1 OR MIT */ +/*
- getopt function definitions for NOLIBC, adapted from musl libc
- Copyright (C) 2005-2020 Rich Felker, et al.
- Copyright (C) 2025 Thomas Weißschuh linux@weissschuh.net
- */
+#ifndef _NOLIBC_GETOPT_H +#define _NOLIBC_GETOPT_H
+struct FILE; +static struct FILE *const stderr; +static int fprintf(struct FILE *stream, const char *fmt, ...);
Is there a particular reason why you had to define these here and include nolibc.h at the bottom instead of doing it the usual way with the include at the top ?
If that's due to a limitation in nolibc, we might want to have a closer look at it before it starts to affect other areas. Also if in the future we have to add some str* dependencies here, it would be easier if we can simply include the file as well.
Doing a regular #include "stdio.h" does fail with the following error:
In file included from sysroot/i386/include/nolibc.h:109, from sysroot/i386/include/errno.h:26, from sysroot/i386/include/stdio.h:12, from harness-selftest.c:3, from nolibc-test.c:5: sysroot/i386/include/getopt.h: In function 'getopt': sysroot/i386/include/getopt.h:72:25: error: implicit declaration of function 'fprintf' [-Werror=implicit-function-declaration] 72 | fprintf(stderr, "%s: unrecognized option: %c\n", argv[0], *optchar); | ^~~~~~~ [+ some followup errors]
The include chain is important here. The user code includes "stdio.h", which at the very beginning includes errno.h->nolibc.h->getopt.h. Now getopt.h tries to use the definitions from stdio.h. However as stdio.h was the entrypoint and is not yet fully parsed, these definitions are not yet available.
+__attribute__((weak,unused,section(".data.nolibc_getopt"))) +char *optarg; +__attribute__((weak,unused,section(".data.nolibc_getopt"))) +int optind = 1; +__attribute__((weak,unused,section(".data.nolibc_getopt"))) +int opterr = 1; +__attribute__((weak,unused,section(".data.nolibc_getopt"))) +int optopt; +__attribute__((weak,unused,section(".data.nolibc_getopt"))) +int __optpos;
I think that for better readability, you'd need to either place them on the same line, or leave a blank line between each declaration.
Ack.
+static __inline__ +int getopt(int argc, char * const argv[], const char *optstring)
It would be better marked with the usual unused attribute. That's a bit large for inlining, and I'm not convinced that the compiler will see any opportunity for simplifying it given that it acts on a list of actions taken from a string.
Ack.