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@xxxxxxxxxxxxxx>
> > + */
> > +
> > +#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.
