Re: [PATCH 24/32] tools/nolibc: add getopt()

[Willy Tarreau]

On Wed, Mar 05, 2025 at 08:25:14AM +0100, Thomas Weißschuh wrote:
> 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.

OK got it, the usual includes dependency mess when it comes to inline
code (here it's static but it's the same) :-(

In the early days I had thought about placing everything in to nolibc.h
and making the standard include files just stubs that would include it,
but I didn't pursue that direction since I had not reached the point of
the problems.

Maybe for the long term we'll have to reopen that reflexion. We could
even have:

  nolibc.h:
     #include "nolibc-types.h"
     #include "nolibc-stdio.h"
     #include "nolibc-stdlib.h"
     ... etc

  and stdio.h, stdlib, etc:
     #include "nolibc.h"

That could be a clean and non-invasive change that would make sure we
always include everything we need in the desired order. If we still
end up with trouble due to some cross-references (since statics are
painful for that), then it becomes possible to have extra -proto.h
files to only declare types and prototypes, not inlines, and that
will be included first.

Let's discuss that later, thanks for explaining!
Willy