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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250304075429.GB9911@1wt.eu>
Date: Tue, 4 Mar 2025 08:54:29 +0100
From: Willy Tarreau <w@....eu>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: Shuah Khan <shuah@...nel.org>, Shuah Khan <skhan@...uxfoundation.org>,
        Thomas Weißschuh <linux@...ssschuh.net>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 24/32] tools/nolibc: add getopt()

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@...ssschuh.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.

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

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

Willy