From d512d3c6d30be6697f36bf042f95816cb6ad36c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 May 2026 20:32:29 +0200 Subject: [PATCH] treewide: get rid of remaing getopt/getopt_long stuff --- .clang-tidy | 1 - src/basic/argv-util.c | 2 +- src/include/musl/getopt.h | 29 -- src/include/musl/unistd.h | 8 - src/libc/musl/getopt.c | 100 ------- src/libc/musl/meson.build | 1 - src/shared/parse-argument.c | 10 +- src/shared/verbs.c | 2 - src/test/meson.build | 1 - src/test/test-getopt.c | 516 ------------------------------------ src/udev/udev-builtin.c | 2 - 11 files changed, 6 insertions(+), 666 deletions(-) delete mode 100644 src/include/musl/getopt.h delete mode 100644 src/libc/musl/getopt.c delete mode 100644 src/test/test-getopt.c diff --git a/.clang-tidy b/.clang-tidy index 8d69a69cbf..c784c9532b 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -38,7 +38,6 @@ CheckOptions: # of them related to musl). misc-include-cleaner.IgnoreHeaders: ' endian\.h; - getopt\.h; sys/stat\.h; sys/statvfs\.h; sys/syscall\.h; diff --git a/src/basic/argv-util.c b/src/basic/argv-util.c index 0ff2fd9bcb..10f8cd86f4 100644 --- a/src/basic/argv-util.c +++ b/src/basic/argv-util.c @@ -71,7 +71,7 @@ bool argv_looks_like_help(int argc, char **argv) { char **l; /* Scans the command line for indications the user asks for help. This is supposed to be called by - * tools that do not implement getopt() style command line parsing because they are not primarily + * tools that do not implement getopt()-style command line parsing because they are not primarily * user-facing. Detects four ways of asking for help: * * 1. Passing zero arguments diff --git a/src/include/musl/getopt.h b/src/include/musl/getopt.h deleted file mode 100644 index 6aed1dfd26..0000000000 --- a/src/include/musl/getopt.h +++ /dev/null @@ -1,29 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -/* getopt() is provided both in getopt.h and unistd.h. Hence, we need to tentatively undefine it. */ -#undef getopt - -#include_next - -/* musl's getopt() always behaves POSIXLY_CORRECT mode, and stops parsing arguments when a non-option string - * found. Let's always use getopt_long(). */ -int getopt_fix(int argc, char * const *argv, const char *optstring); -#define getopt(argc, argv, optstring) getopt_fix(argc, argv, optstring) - -/* musl's getopt_long() behaves something different in handling optional arguments. - * ======== - * $ journalctl _PID=1 _COMM=systemd --since 19:19:01 -n all --follow - * Failed to add match 'all': Invalid argument - * ======== - * Here, we introduce getopt_long_fix() that reorders the passed arguments to make getopt_long() provided by - * musl works as what we expect. */ -int getopt_long_fix( - int argc, - char * const *argv, - const char *optstring, - const struct option *longopts, - int *longindex); - -#define getopt_long(argc, argv, optstring, longopts, longindex) \ - getopt_long_fix(argc, argv, optstring, longopts, longindex) diff --git a/src/include/musl/unistd.h b/src/include/musl/unistd.h index e58fee356a..175661442d 100644 --- a/src/include/musl/unistd.h +++ b/src/include/musl/unistd.h @@ -1,16 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -/* getopt() is provided both in getopt.h and unistd.h. Hence, we need to tentatively undefine it. */ -#undef getopt - #include_next -/* musl's getopt() always behaves POSIXLY_CORRECT mode, and stops parsing arguments when a non-option string - * found. Let's always use getopt_long(). */ -int getopt_fix(int argc, char * const *argv, const char *optstring); -#define getopt(argc, argv, optstring) getopt_fix(argc, argv, optstring) - int missing_close_range(unsigned first_fd, unsigned end_fd, unsigned flags); #define close_range missing_close_range diff --git a/src/libc/musl/getopt.c b/src/libc/musl/getopt.c deleted file mode 100644 index 15c3afa49b..0000000000 --- a/src/libc/musl/getopt.c +++ /dev/null @@ -1,100 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include -#include -#include - -static int first_non_opt = 0, last_non_opt = 0; -static bool non_opt_found = false, dash_dash = false; - -static void shift(char * const *argv, int start, int end) { - char **av = (char**) argv; - char *saved = av[end]; - - for (int i = end; i > start; i--) - av[i] = av[i - 1]; - - av[start] = saved; -} - -static void exchange(int argc, char * const *argv) { - /* input: - * - * first_non_opt last_non_opt optind - * | | | - * v v v - * aaaaa bbbbb ccccc --prev-opt prev-opt-arg ddddd --next-opt - * - * output: - * first_non_opt last_non_opt optind - * | | | - * v v v - * --prev-opt prev-opt-arg aaaaa bbbbb ccccc ddddd --next-opt - */ - - /* First, move previous arguments. */ - int c = optind - 1 - last_non_opt; - if (c > 0) { - for (int i = 0; i < c; i++) - shift(argv, first_non_opt, optind - 1); - first_non_opt += c; - last_non_opt += c; - } - - /* Then, skip entries that do not start with '-'. */ - while (optind < argc && (argv[optind][0] != '-' || argv[optind][1] == '\0')) { - if (!non_opt_found) { - first_non_opt = optind; - non_opt_found = true; - } - last_non_opt = optind; - optind++; - } -} - -int getopt_long_fix( - int argc, - char * const *argv, - const char *optstring, - const struct option *longopts, - int *longindex) { - - int r; - - if (optind == 0 || first_non_opt == 0 || last_non_opt == 0) { - /* initialize musl's internal variables. */ - (void) (getopt_long)(/* argc= */ -1, /* argv= */ NULL, /* optstring= */ NULL, /* longopts= */ NULL, /* longindex= */ NULL); - first_non_opt = last_non_opt = 1; - non_opt_found = dash_dash = false; - } - - if (first_non_opt >= argc || last_non_opt >= argc || optind > argc || dash_dash) - return -1; - - /* Do not shuffle arguments when optstring starts with '+' or '-'. */ - if (!optstring || optstring[0] == '+' || optstring[0] == '-') - return (getopt_long)(argc, argv, optstring, longopts, longindex); - - exchange(argc, argv); - - if (optind < argc && strcmp(argv[optind], "--") == 0) { - if (first_non_opt < optind) - shift(argv, first_non_opt, optind); - first_non_opt++; - optind++; - dash_dash = true; - if (non_opt_found) - optind = first_non_opt; - return -1; - } - - r = (getopt_long)(argc, argv, optstring, longopts, longindex); - if (r < 0 && non_opt_found) - optind = first_non_opt; - - return r; -} - -int getopt_fix(int argc, char * const *argv, const char *optstring) { - return getopt_long_fix(argc, argv, optstring, /* longopts= */ NULL, /* longindex= */ NULL); -} diff --git a/src/libc/musl/meson.build b/src/libc/musl/meson.build index acda973b8d..a06216f063 100644 --- a/src/libc/musl/meson.build +++ b/src/libc/musl/meson.build @@ -6,7 +6,6 @@ endif libc_wrapper_sources += files( 'crypt.c', - 'getopt.c', 'printf.c', 'stdio.c', 'stdlib.c', diff --git a/src/shared/parse-argument.c b/src/shared/parse-argument.c index 0d4bfa4c91..bfd21a910e 100644 --- a/src/shared/parse-argument.c +++ b/src/shared/parse-argument.c @@ -31,7 +31,7 @@ int parse_boolean_argument(const char *optname, const char *s, bool *ret) { *ret = r; return r; } else { - /* s may be NULL. This is controlled by getopt_long() parameters. */ + /* s may be NULL. This is controlled by OPTION flags. */ if (ret) *ret = true; return true; @@ -42,10 +42,10 @@ int parse_tristate_argument_with_auto(const char *optname, const char *s, int *r int r; assert(optname); - assert(s); /* We refuse NULL optarg here, since that would be ambiguous on cmdline: - for --enable-a[=BOOL], --enable-a is intuitively interpreted as true rather than "auto" - (parse_boolean_argument() does exactly that). IOW, tristate options should require - arguments. */ + assert(s); /* We refuse NULL arg here, since that would be ambiguous on cmdline: for + * --enable-a[=BOOL], --enable-a is intuitively interpreted as true rather than "auto" + * (parse_boolean_argument() does exactly that). IOW, tristate options should require + * arguments. */ r = parse_tristate_full(s, "auto", ret); if (r < 0) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index e9e78c5b72..34e8a897d6 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include - #include "env-util.h" #include "format-table.h" #include "log.h" diff --git a/src/test/meson.build b/src/test/meson.build index 20960d0b95..4871c5517f 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -111,7 +111,6 @@ simple_tests += files( 'test-format-util.c', 'test-fs-util.c', 'test-fstab-util.c', - 'test-getopt.c', 'test-glob-util.c', 'test-gpt.c', 'test-gunicode.c', diff --git a/src/test/test-getopt.c b/src/test/test-getopt.c deleted file mode 100644 index 0ba1c678f2..0000000000 --- a/src/test/test-getopt.c +++ /dev/null @@ -1,516 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include - -#include "strv.h" -#include "tests.h" - -typedef struct Entry { - int opt; - const char *argument; - const char *nextarg; -} Entry; - -static void test_getopt_long_one( - char **argv, - const char *optstring, - const struct option *longopts, - const Entry *entries, - char **remaining) { - - _cleanup_free_ char *joined = strv_join(argv, ", "); - log_debug("/* %s(%s) */", __func__, joined); - - _cleanup_free_ char *saved_argv0 = NULL; - ASSERT_NOT_NULL(saved_argv0 = strdup(argv[0])); - - int c, argc = strv_length(argv); - size_t i = 0, n_entries = 0; - - for (const Entry *e = entries; e && e->opt != 0; e++) - n_entries++; - - optind = 0; - while ((c = getopt_long(argc, argv, optstring, longopts, NULL)) >= 0) { - if (c < 0x100) - log_debug("%c: %s", c, strna(optarg)); - else - log_debug("0x%x: %s", (unsigned) c, strna(optarg)); - - ASSERT_LT(i, n_entries); - ASSERT_EQ(c, entries[i].opt); - ASSERT_STREQ(optarg, entries[i].argument); - if (entries[i].nextarg) - ASSERT_STREQ(argv[optind], entries[i].nextarg); - i++; - } - - ASSERT_EQ(i, n_entries); - ASSERT_LE(optind, argc); - ASSERT_EQ(argc - optind, (int) strv_length(remaining)); - for (int j = optind; j < argc; j++) - ASSERT_STREQ(argv[j], remaining[j - optind]); - ASSERT_STREQ(argv[0], saved_argv0); -} - -TEST(getopt_long) { - enum { - ARG_VERSION = 0x100, - ARG_REQUIRED, - ARG_OPTIONAL, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version" , no_argument, NULL, ARG_VERSION }, - { "required1", required_argument, NULL, 'r' }, - { "required2", required_argument, NULL, ARG_REQUIRED }, - { "optional1", optional_argument, NULL, 'o' }, - { "optional2", optional_argument, NULL, ARG_OPTIONAL }, - {}, - }; - - test_getopt_long_one(STRV_MAKE("arg0"), - "hr:o::", options, - NULL, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "string3", - "string4"), - "hr:o::", options, - NULL, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "--", - "string1", - "--help", - "-h", - "string4"), - "hr:o::", options, - NULL, - STRV_MAKE("string1", - "--help", - "-h", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "--", - "--", - "string4"), - "hr:o::", options, - NULL, - STRV_MAKE("string1", - "string2", - "--", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "string3", - "string4", - "--"), - "hr:o::", options, - NULL, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "--help"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "-h"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "--help", - "string1", - "string2", - "string3", - "string4"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "-h", - "string1", - "string2", - "string3", - "string4"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "--help", - "string3", - "string4"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "-h", - "string3", - "string4"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "string3", - "string4", - "--help"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "string3", - "string4", - "-h"), - "hr:o::", options, - (Entry[]) { - { 'h', NULL }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4")); - - test_getopt_long_one(STRV_MAKE("arg0", - "--required1", "reqarg1"), - "hr:o::", options, - (Entry[]) { - { 'r', "reqarg1" }, - {} - }, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "-r", "reqarg1"), - "hr:o::", options, - (Entry[]) { - { 'r', "reqarg1" }, - {} - }, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "string2", - "-r", "reqarg1"), - "hr:o::", options, - (Entry[]) { - { 'r', "reqarg1" }, - {} - }, - STRV_MAKE("string1", - "string2")); - - test_getopt_long_one(STRV_MAKE("arg0", - "--optional1=optarg1"), - "hr:o::", options, - (Entry[]) { - { 'o', "optarg1" }, - {} - }, - NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "--optional1", "string1"), - "hr:o::", options, - (Entry[]) { - { 'o', NULL, "string1" }, - {} - }, - STRV_MAKE("string1")); - - test_getopt_long_one(STRV_MAKE("arg0", - "-ooptarg1"), - "hr:o::", options, - (Entry[]) { - { 'o', "optarg1" }, - {} - }, NULL); - - test_getopt_long_one(STRV_MAKE("arg0", - "-o", "string1"), - "hr:o::", options, - (Entry[]) { - { 'o', NULL, "string1" }, - {} - }, - STRV_MAKE("string1")); - - test_getopt_long_one(STRV_MAKE("arg0", - "string1", - "--help", - "--version", - "string2", - "--required1", "reqarg1", - "--required2", "reqarg2", - "--required1=reqarg3", - "--required2=reqarg4", - "string3", - "--optional1", "string4", - "--optional2", "string5", - "--optional1=optarg1", - "--optional2=optarg2", - "-h", - "-r", "reqarg5", - "-rreqarg6", - "-ooptarg3", - "-o", - "string6", - "-o", - "-h", - "-o", - "--help", - "string7", - "-hooptarg4", - "-hrreqarg6", - "--", - "--help", - "--required1", - "--optional1"), - "hr:o::", options, - (Entry[]) { - { 'h' }, - { ARG_VERSION }, - { 'r', "reqarg1" }, - { ARG_REQUIRED, "reqarg2" }, - { 'r', "reqarg3" }, - { ARG_REQUIRED, "reqarg4" }, - { 'o', NULL, "string4" }, - { ARG_OPTIONAL, NULL, "string5" }, - { 'o', "optarg1" }, - { ARG_OPTIONAL, "optarg2" }, - { 'h' }, - { 'r', "reqarg5" }, - { 'r', "reqarg6" }, - { 'o', "optarg3" }, - { 'o', NULL, "string6" }, - { 'o', NULL, "-h" }, - { 'h' }, - { 'o', NULL, "--help" }, - { 'h' }, - { 'h' }, - { 'o', "optarg4" }, - { 'h' }, - { 'r', "reqarg6" }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string4", - "string5", - "string6", - "string7", - "--help", - "--required1", - "--optional1")); -} -static void test_getopt_one( - char **argv, - const char *optstring, - const Entry *entries, - char **remaining) { - - _cleanup_free_ char *joined = strv_join(argv, ", "); - log_debug("/* %s(%s) */", __func__, joined); - - _cleanup_free_ char *saved_argv0 = NULL; - ASSERT_NOT_NULL(saved_argv0 = strdup(argv[0])); - - int c, argc = strv_length(argv); - size_t i = 0, n_entries = 0; - - for (const Entry *e = entries; e && e->opt != 0; e++) - n_entries++; - - optind = 0; - while ((c = getopt(argc, argv, optstring)) >= 0) { - log_debug("%c: %s", c, strna(optarg)); - - ASSERT_LT(i, n_entries); - ASSERT_EQ(c, entries[i].opt); - ASSERT_STREQ(optarg, entries[i].argument); - if (entries[i].nextarg) - ASSERT_STREQ(argv[optind], entries[i].nextarg); - i++; - } - - ASSERT_EQ(i, n_entries); - ASSERT_LE(optind, argc); - ASSERT_EQ(argc - optind, (int) strv_length(remaining)); - for (int j = optind; j < argc; j++) - ASSERT_STREQ(argv[j], remaining[j - optind]); - ASSERT_STREQ(argv[0], saved_argv0); -} - -TEST(getopt) { - test_getopt_one(STRV_MAKE("arg0"), - "hr:o::", - NULL, - NULL); - - test_getopt_one(STRV_MAKE("arg0", - "string1", - "string2"), - "hr:o::", - NULL, - STRV_MAKE("string1", - "string2")); - - test_getopt_one(STRV_MAKE("arg0", - "-h"), - "hr:o::", - (Entry[]) { - { 'h', NULL }, - {} - }, - NULL); - - test_getopt_one(STRV_MAKE("arg0", - "-r", "reqarg1"), - "hr:o::", - (Entry[]) { - { 'r', "reqarg1" }, - {} - }, - NULL); - - test_getopt_one(STRV_MAKE("arg0", - "string1", - "string2", - "-r", "reqarg1"), - "hr:o::", - (Entry[]) { - { 'r', "reqarg1" }, - {} - }, - STRV_MAKE("string1", - "string2")); - - test_getopt_one(STRV_MAKE("arg0", - "-ooptarg1"), - "hr:o::", - (Entry[]) { - { 'o', "optarg1" }, - {} - }, - NULL); - - test_getopt_one(STRV_MAKE("arg0", - "-o", "string1"), - "hr:o::", - (Entry[]) { - { 'o', NULL, "string1" }, - {} - }, - STRV_MAKE("string1")); - - test_getopt_one(STRV_MAKE("arg0", - "string1", - "string2", - "string3", - "-h", - "-r", "reqarg5", - "-rreqarg6", - "-ooptarg3", - "-o", - "string6", - "-o", - "-h", - "-o", - "string7", - "-hooptarg4", - "-hrreqarg6"), - "hr:o::", - (Entry[]) { - { 'h' }, - { 'r', "reqarg5" }, - { 'r', "reqarg6" }, - { 'o', "optarg3" }, - { 'o', NULL, "string6" }, - { 'o', NULL, "-h" }, - { 'h' }, - { 'o', NULL, "string7" }, - { 'h' }, - { 'o', "optarg4" }, - { 'h' }, - { 'r', "reqarg6" }, - {} - }, - STRV_MAKE("string1", - "string2", - "string3", - "string6", - "string7")); -} - -DEFINE_TEST_MAIN(LOG_DEBUG); diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c index 80a8700a62..fbed496c4e 100644 --- a/src/udev/udev-builtin.c +++ b/src/udev/udev-builtin.c @@ -127,8 +127,6 @@ int udev_builtin_run(UdevEvent *event, UdevBuiltinCommand cmd, const char *comma if (r < 0) return r; - /* we need '0' here to reset the internal state */ - optind = 0; return builtins[cmd]->cmd(event, strv_length(argv), argv); }