From eafd3e6a7d1b5c4bd46266a7991ce83d8995fb81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 24 Feb 2026 16:13:06 +0100 Subject: [PATCH 01/15] Add "option parser" infrastracture that helps with cmdline option parsing The basic idea is that we'll have "one source of truth" for the list of options. Currently, this is split between: 1. struct option options[] array for long options 2. the short option parameter to getopt_long() 3. --help so it is easy to forget to add or update one of those places where appropriate. An option is defined through a macro that includes the option short and long codes, and also the metavar and help. Those four items can be used to generate the help string automatically. The code is easier to read when various parts are written in the same order. We can define common options through a macro in the header file, reducing boilerplate repeated in different files. Over time, if we discover that the same pattern is used in multiple files, we can add another "common option". The macro is defined in a way that the editor can indent it like a normal case statement. The error message for ambiguous options is formatted a bit differently: $ systemd-id128 --no- systemd-id128: option '--no-' is ambiguous; possibilities: '--no-pager' '--no-legend' $ build/systemd-id128 --no- option '--no-' is ambiguous; possibilities: --no-pager, --no-legend I think the formatting without commas is ugly, but OTOH, the quotes around option names are superfluous, real option names are easy to distinguish. --- src/shared/meson.build | 1 + src/shared/options.c | 319 +++++++++++++++++++++++++++++++++++++++++ src/shared/options.h | 101 +++++++++++++ src/shared/verbs.c | 37 ++--- src/shared/verbs.h | 1 + 5 files changed, 442 insertions(+), 17 deletions(-) create mode 100644 src/shared/options.c create mode 100644 src/shared/options.h diff --git a/src/shared/meson.build b/src/shared/meson.build index bbc0307999..e25f855c71 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -143,6 +143,7 @@ shared_sources = files( 'numa-util.c', 'open-file.c', 'openssl-util.c', + 'options.c', 'osc-context.c', 'output-mode.c', 'pager.c', diff --git a/src/shared/options.c b/src/shared/options.c new file mode 100644 index 0000000000..c5e7057bf8 --- /dev/null +++ b/src/shared/options.c @@ -0,0 +1,319 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "format-table.h" +#include "log.h" +#include "options.h" +#include "stdio-util.h" +#include "string-util.h" +#include "strv.h" + +static bool option_takes_arg(const Option *opt) { + return ASSERT_PTR(opt)->metavar; +} + +static bool option_arg_optional(const Option *opt) { + return option_takes_arg(opt) && FLAGS_SET(opt->flags, OPTION_OPTIONAL_ARG); +} + +static bool option_arg_required(const Option *opt) { + return option_takes_arg(opt) && !FLAGS_SET(opt->flags, OPTION_OPTIONAL_ARG); +} + +static bool option_is_metadata(const Option *opt) { + /* A metadata entry that is not a real option, like the group marker */ + return FLAGS_SET(ASSERT_PTR(opt)->flags, OPTION_GROUP_MARKER); +} + +static void kill_arg(char* argv[], int argc, int index) { + assert(index < argc); + assert(!argv[argc]); + + /* Eliminate argv[index] */ + memmove(argv + index, argv + index + 1, (argc - index) * sizeof(char*)); +} + +static void shift_arg(char* argv[], int target, int source) { + assert(argv); + assert(target <= source); + + /* Move argv[source] before argv[target], shifting arguments inbetween */ + char *saved = argv[source]; + memmove(argv + target + 1, argv + target, (source - target) * sizeof(char*)); + argv[target] = saved; +} + +static int partial_match_error( + const Option options[], + const Option options_end[], + const char *optname, + unsigned n_partial_matches) { + int r; + + assert(startswith(ASSERT_PTR(optname), "--")); + assert(n_partial_matches >= 2); + + /* Find options that match the prefix */ + _cleanup_strv_free_ char **s = NULL; + for (const Option* option = options; option < options_end; option++) + if (!option_is_metadata(option) && + option->long_code && + startswith(option->long_code, optname + 2)) { + + r = strv_extendf(&s, "--%s", option->long_code); + if (r < 0) + return log_error_errno(r, "Failed to format message: %m"); + } + + assert(strv_length(s) == n_partial_matches); + + _cleanup_free_ char *p = strv_join_full(s, ", ", /* prefix= */ NULL, /* escape_separator= */ false); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: option '%s' is ambiguous; possibilities: %s", + program_invocation_short_name, optname, strnull(p)); +} + +int option_parse( + const Option options[], + const Option options_end[], + OptionParser *state, + int argc, char *argv[], + const Option **ret_option, + const char **ret_arg) { + + assert(ret_arg); + + /* Check and initialize */ + if (state->optind == 0) { + if (argc < 1 || strv_isempty(argv)) + return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "argv cannot be empty"); + + *state = (OptionParser) { + .optind = 1, + .positional_offset = 1, + }; + } + + /* Look for the next option */ + + const Option *option; + const char *optname = NULL, *optval = NULL; + _cleanup_free_ char *_optname = NULL; /* allocated option name */ + bool separate_optval = false; + + if (state->short_option_offset == 0) { + /* Skip over non-option parameters */ + for (;;) { + if (state->optind == argc) + return 0; + + if (streq(argv[state->optind], "--")) { + /* No more options. Eliminate "--" so that the list of positional args is clean. */ + kill_arg(argv, argc, state->optind); + return 0; + } + + if (!state->parsing_stopped && + argv[state->optind][0] == '-' && + argv[state->optind][1] != '\0') + /* Looks like we found an option parameter */ + break; + + state->optind++; + } + + /* Find matching option entry. + * First, figure out if we have a long option or a short option. */ + assert(argv[state->optind][0] == '-'); + + if (argv[state->optind][1] == '-') { + /* We have a long option. */ + char *eq = strchr(argv[state->optind], '='); + if (eq) { + optname = _optname = strndup(argv[state->optind], eq - argv[state->optind]); + if (!_optname) + return log_oom(); + + /* joined argument */ + optval = eq + 1; + } else + /* argument (if any) is separate */ + optname = argv[state->optind]; + + const Option *last_partial = NULL; + unsigned n_partial_matches = 0; /* The commandline option matches a defined prefix. */ + + for (option = options;; option++) { + if (option >= options_end) { + if (n_partial_matches == 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: unrecognized option '%s'", + program_invocation_short_name, optname); + if (n_partial_matches > 1) + return partial_match_error(options, options_end, optname, n_partial_matches); + + /* just one partial — good */ + option = last_partial; + break; + } + + if (option_is_metadata(option) || !option->long_code) + continue; + + /* Check if the parameter forms a prefix of the option name */ + const char *rest = startswith(option->long_code, optname + 2); + if (!rest) + continue; + if (isempty(rest)) + /* exact match */ + break; + /* partial match */ + last_partial = option; + n_partial_matches++; + } + } else + /* We have a short option */ + state->short_option_offset = 1; + } + + if (state->short_option_offset > 0) { + char optchar = argv[state->optind][state->short_option_offset]; + + if (asprintf(&_optname, "-%c", optchar) < 0) + return log_oom(); + optname = _optname; + + for (option = options;; option++) { + if (option >= options_end) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: unrecognized option '%s'", + program_invocation_short_name, optname); + + if (option_is_metadata(option) || optchar != option->short_code) + continue; + + const char *rest = argv[state->optind] + state->short_option_offset + 1; + + if (option_takes_arg(option) && !isempty(rest)) { + /* The rest of this parameter is the value. */ + optval = rest; + state->short_option_offset = 0; + } else if (isempty(rest)) + state->short_option_offset = 0; + else + state->short_option_offset++; + + break; + } + } + + if (optval && !option_takes_arg(option)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: option '%s' doesn't allow an argument", + program_invocation_short_name, optname); + if (!optval && option_arg_required(option)) { + if (!argv[state->optind + 1]) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: option '%s' requires an argument", + program_invocation_short_name, optname); + optval = argv[state->optind + 1]; + separate_optval = true; + } + + if (state->short_option_offset == 0) { + /* We're done with this option. Adjust the array and position. */ + shift_arg(argv, state->positional_offset++, state->optind++); + if (separate_optval) + shift_arg(argv, state->positional_offset++, state->optind++); + } + + if (FLAGS_SET(option->flags, OPTION_STOPS_PARSING)) + state->parsing_stopped = true; + + if (ret_option) + /* Return the matched Option structure to allow the caller to "know" what was matched */ + *ret_option = option; + *ret_arg = optval; + return option->id; +} + +char** option_parser_get_args(OptionParser *state, int argc, char *argv[]) { + /* Returns positional args as a strv. + * If "--" was found, it has been removed. */ + + assert(state->optind > 0); + return argv + state->positional_offset; +} + +int _option_parser_get_help_table( + const Option options[], + const Option options_end[], + const char *group, + Table **ret) { + int r; + + assert(ret); + + _cleanup_(table_unrefp) Table *table = table_new("names", "help"); + if (!table) + return log_oom(); + + bool in_group = group == NULL; /* Are we currently in the section on the array that forms + * group ? The first part is the default group, so + * the group was not specified, we are in. */ + + for (const Option *opt = options; opt < options_end; opt++) { + bool group_marker = FLAGS_SET(opt->flags, OPTION_GROUP_MARKER); + if (!in_group) { + in_group = group_marker && streq(group, opt->long_code); + continue; + } + if (group_marker) + break; /* End of group */ + + assert(!option_is_metadata(opt)); + + if (!opt->help) + /* No help string — we do not show the option */ + continue; + + char sc[3] = " "; + if (opt->short_code != 0) + xsprintf(sc, "-%c", opt->short_code); + + /* We indent the option string by two spaces. We could set the minimum cell width and + * right-align for a similar result, but that'd be more work. This is only used for + * display. + * + * "=" is shown only when a long option is defined: -l --long=ARG, --long=ARG, -s ARG. + */ + bool need_eq = option_takes_arg(opt) && opt->long_code; + _cleanup_free_ char *s = strjoin( + " ", + sc, + " ", + opt->long_code ? "--" : "", + strempty(opt->long_code), + option_arg_optional(opt) ? "[" : "", + need_eq ? "=" : "", + strempty(opt->metavar), + option_arg_optional(opt) ? "]" : ""); + if (!s) + return log_oom(); + + r = table_add_many(table, TABLE_STRING, s); + if (r < 0) + return table_log_add_error(r); + + _cleanup_strv_free_ char **t = strv_split(opt->help, /* separators= */ NULL); + if (!t) + return log_oom(); + + r = table_add_many(table, TABLE_STRV_WRAPPED, t); + if (r < 0) + return table_log_add_error(r); + } + + table_set_header(table, false); + *ret = TAKE_PTR(table); + return 0; +} diff --git a/src/shared/options.h b/src/shared/options.h new file mode 100644 index 0000000000..f548538bd0 --- /dev/null +++ b/src/shared/options.h @@ -0,0 +1,101 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "shared-forward.h" + +typedef enum OptionFlags { + OPTION_OPTIONAL_ARG = 1U << 0, /* Same as optional_argument in getopt */ + OPTION_STOPS_PARSING = 1U << 1, /* This option acts like "--" */ + OPTION_GROUP_MARKER = 1U << 2, /* Fake option entry to separate groups */ +} OptionFlags; + +typedef struct Option { + int id; + OptionFlags flags; + char short_code; + const char *long_code; + const char *metavar; + const char *help; +} Option; + +#define _OPTION(counter, fl, sc, lc, mv, h) \ + _section_("SYSTEMD_OPTIONS") \ + _alignptr_ \ + _used_ \ + _retain_ \ + _variable_no_sanitize_address_ \ + static const Option CONCATENATE(option, counter) = { \ + .id = 0x100 + counter, \ + .flags = fl, \ + .short_code = sc, \ + .long_code = lc, \ + .metavar = mv, \ + .help = h, \ + }; \ + case (0x100 + counter) + +/* Magic entry in the table (which will not be returned) that designates the start of the group . + * The define is structured as 'case' so that it can be followed by ':' and indented appropriately. + */ +#define OPTION_GROUP(gr) \ + _OPTION(__COUNTER__, OPTION_GROUP_MARKER, /* sc= */ 0, /* lc= */ gr, /* mv= */ NULL, /* h= */ NULL) + +#define OPTION_FULL(fl, sc, lc, mv, h) _OPTION(__COUNTER__, fl, sc, lc, mv, h) +#define OPTION(sc, lc, mv, h) OPTION_FULL(/* fl= */ 0, sc, lc, mv, h) +#define OPTION_LONG(lc, mv, h) OPTION(/* sc= */ 0, lc, mv, h) +#define OPTION_SHORT(sc, mv, h) OPTION(sc, /* lc= */ NULL, mv, h) + +#define OPTION_COMMON_HELP \ + OPTION('h', "help", NULL, "Show this help") +#define OPTION_COMMON_VERSION \ + OPTION_LONG("version", NULL, "Show package version") +#define OPTION_COMMON_NO_PAGER \ + OPTION_LONG("no-pager", NULL, "Do not start a pager") +#define OPTION_COMMON_NO_LEGEND \ + OPTION_LONG("no-legend", NULL, "Do not show headers and footers") +#define OPTION_COMMON_JSON \ + OPTION_LONG("json", "FORMAT", "Generate JSON output (pretty, short, or off)") + +/* This is magically mapped to the beginning and end of the section */ +extern const Option __start_SYSTEMD_OPTIONS[]; +extern const Option __stop_SYSTEMD_OPTIONS[]; + +typedef struct OptionParser { + int optind; /* Position of the parameter being handled. + * 0 → option parsing hasn't been started yet. */ + int short_option_offset; /* Set when we're parsing an argument with one or more short options. + * 0 → we're not parsing short options. */ + int positional_offset; /* Offset to where positional parameters are. After processing has been + * finished, all options and their args are to the left of this offset. */ + bool parsing_stopped; /* We processed "--" or an option that terminates option parsing. */ +} OptionParser; + +int option_parse( + const Option options[], + const Option options_end[], + OptionParser *state, + int argc, char *argv[], + const Option **ret_option, + const char **ret_arg); + +/* Iterate over options. */ +#define FOREACH_OPTION_FULL(parser, opt, argc, argv, ret_o, ret_a, on_error) \ + for (int opt; (opt = option_parse(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, parser, argc, argv, ret_o, ret_a)) != 0; ) \ + if (opt < 0) { \ + on_error; \ + break; \ + } else + +#define FOREACH_OPTION(parser, opt, argc, argv, ret_a, on_error) \ + FOREACH_OPTION_FULL(parser, opt, argc, argv, /* ret_o= */ NULL, ret_a, on_error) + +char** option_parser_get_args(OptionParser *state, int argc, char *argv[]); +int _option_parser_get_help_table( + const Option options[], + const Option options_end[], + const char *group, + Table **ret); +#define option_parser_get_help_table_group(group, ret) \ + _option_parser_get_help_table(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, group, ret) +#define option_parser_get_help_table(ret) \ + option_parser_get_help_table_group(/* group= */ NULL, ret) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index c6d35913b4..8c174a6c88 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -68,24 +68,17 @@ const Verb* verbs_find_verb(const char *name, const Verb verbs[]) { return NULL; } -int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { - const Verb *verb; - const char *name; - int r, left; +int dispatch_verb_with_args(char **args, const Verb verbs[], void *userdata) { + int r; assert(verbs); assert(verbs[0].dispatch); assert(verbs[0].verb); - assert(argc >= 0); - assert(argv); - assert(argc >= optind); - left = argc - optind; - argv += optind; - optind = 0; - name = argv[0]; + const char *name = args ? args[0] : NULL; + size_t left = strv_length(args); - verb = verbs_find_verb(name, verbs); + const Verb *verb = verbs_find_verb(name, verbs); if (!verb) { _cleanup_strv_free_ char **verb_strv = NULL; @@ -120,12 +113,10 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { if (!name) left = 1; - if (verb->min_args != VERB_ANY && - (unsigned) left < verb->min_args) + if (verb->min_args != VERB_ANY && left < verb->min_args) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Too few arguments."); - if (verb->max_args != VERB_ANY && - (unsigned) left > verb->max_args) + if (verb->max_args != VERB_ANY && left > verb->max_args) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Too many arguments."); if ((verb->flags & VERB_ONLINE_ONLY) && running_in_chroot_or_offline()) { @@ -136,5 +127,17 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { if (!name) return verb->dispatch(1, STRV_MAKE(verb->verb), verb->data, userdata); - return verb->dispatch(left, argv, verb->data, userdata); + assert(left < INT_MAX); /* args are derived from argc+argv, so their size must fit in an int. */ + return verb->dispatch(left, args, verb->data, userdata); +} + +int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { + /* getopt wrapper for _dispatch_verb_with_args. + * TBD: remove this function when all programs with verbs have been converted. */ + + assert(argc >= 0); + assert(argv); + assert(argc >= optind); + + return dispatch_verb_with_args(strv_skip(argv, optind), verbs, userdata); } diff --git a/src/shared/verbs.h b/src/shared/verbs.h index e330156318..febb8ccfc2 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -23,4 +23,5 @@ bool running_in_chroot_or_offline(void); bool should_bypass(const char *env_prefix); const Verb* verbs_find_verb(const char *name, const Verb verbs[]); +int dispatch_verb_with_args(char **args, const Verb verbs[], void *userdata); int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata); From b6bc0948cd2f5962b77008d4337d88630a9eabd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 1 Mar 2026 15:37:24 +0100 Subject: [PATCH 02/15] =?UTF-8?q?options:=20add=20workaround=20for=20spuri?= =?UTF-8?q?ous=20warning=20with=20gcc=2011=E2=80=9313?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gcc 14+ doesn't need this. --- src/shared/options.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/shared/options.c b/src/shared/options.c index c5e7057bf8..09b677f813 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -95,7 +95,7 @@ int option_parse( /* Look for the next option */ - const Option *option; + const Option *option = NULL; /* initialization to appease gcc 13 */ const char *optname = NULL, *optval = NULL; _cleanup_free_ char *_optname = NULL; /* allocated option name */ bool separate_optval = false; @@ -206,6 +206,8 @@ int option_parse( } } + assert(option); + if (optval && !option_takes_arg(option)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s: option '%s' doesn't allow an argument", From cc07d8ab472be24843a3a6f7c8648af8dbbcaa22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 19:10:56 +0100 Subject: [PATCH 03/15] test-option-parser: "translate" test-getopt for the new parser The test cases are the same in both files. To make the test more through, add case where "--" is used more than once and also when options are present after "--". --- src/test/meson.build | 1 + src/test/test-getopt.c | 12 +- src/test/test-options.c | 375 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 382 insertions(+), 6 deletions(-) create mode 100644 src/test/test-options.c diff --git a/src/test/meson.build b/src/test/meson.build index adbcd3c0d4..d11d853f1d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -156,6 +156,7 @@ simple_tests += files( 'test-nsresource.c', 'test-nulstr-util.c', 'test-open-file.c', + 'test-options.c', 'test-ordered-set.c', 'test-os-util.c', 'test-osc-context.c', diff --git a/src/test/test-getopt.c b/src/test/test-getopt.c index e17621af6a..0ba1c678f2 100644 --- a/src/test/test-getopt.c +++ b/src/test/test-getopt.c @@ -90,27 +90,27 @@ TEST(getopt_long) { test_getopt_long_one(STRV_MAKE("arg0", "--", "string1", - "string2", - "string3", + "--help", + "-h", "string4"), "hr:o::", options, NULL, STRV_MAKE("string1", - "string2", - "string3", + "--help", + "-h", "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", diff --git a/src/test/test-options.c b/src/test/test-options.c new file mode 100644 index 0000000000..56e8dd1d61 --- /dev/null +++ b/src/test/test-options.c @@ -0,0 +1,375 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "options.h" +#include "strv.h" +#include "tests.h" + +typedef struct Entry { + const char *long_code; + const char *argument; +} Entry; + +static void test_option_parse_one( + char **argv, + const Option options[], + 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 argc = strv_length(argv); + size_t i = 0, n_options = 0, n_entries = 0; + + for (const Option *o = options; o->short_code != 0 || o->long_code; o++) + n_options++; + + for (const Entry *e = entries; e && e->long_code; e++) + n_entries++; + + OptionParser state = {}; + const Option *opt; + const char *arg; + for (int c; (c = option_parse(options, options + n_options, &state, argc, argv, &opt, &arg)) != 0; ) { + ASSERT_OK(c); + ASSERT_NOT_NULL(opt); + + log_debug("%c %s: %s=%s", + opt->short_code != 0 ? opt->short_code : ' ', + opt->long_code ?: "", + strnull(opt->metavar), strnull(arg)); + + ASSERT_LT(i, n_entries); + ASSERT_TRUE(streq_ptr(opt->long_code, entries[i].long_code)); + ASSERT_TRUE(streq_ptr(arg, entries[i].argument)); + i++; + } + + ASSERT_EQ(i, n_entries); + + char **args = option_parser_get_args(&state, argc, argv); + ASSERT_TRUE(strv_equal(args, remaining)); + ASSERT_STREQ(argv[0], saved_argv0); +} + +TEST(option_parse) { + static const Option options[] = { + { 1, .short_code = 'h', .long_code = "help" }, + { 2, .long_code = "version" }, + { 3, .short_code = 'r', .long_code = "required1", .metavar = "ARG" }, + { 4, .long_code = "required2", .metavar = "ARG" }, + { 5, .short_code = 'o', .long_code = "optional1", .metavar = "ARG", .flags = OPTION_OPTIONAL_ARG }, + { 6, .long_code = "optional2", .metavar = "ARG", .flags = OPTION_OPTIONAL_ARG }, + {} + }; + + test_option_parse_one(STRV_MAKE("arg0"), + options, + NULL, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "string3", + "string4"), + options, + NULL, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "--", + "string1", + "--help", + "-h", + "string4"), + options, + NULL, + STRV_MAKE("string1", + "--help", + "-h", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "--", + "--", + "string4"), + options, + NULL, + STRV_MAKE("string1", + "string2", + "--", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "string3", + "string4", + "--"), + options, + NULL, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "--help"), + options, + (Entry[]) { + { "help" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "-h"), + options, + (Entry[]) { + { "help" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "--help", + "string1", + "string2", + "string3", + "string4"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "-h", + "string1", + "string2", + "string3", + "string4"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "--help", + "string3", + "string4"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "-h", + "string3", + "string4"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "string3", + "string4", + "--help"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "string3", + "string4", + "-h"), + options, + (Entry[]) { + { "help" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4")); + + test_option_parse_one(STRV_MAKE("arg0", + "--required1", "reqarg1"), + options, + (Entry[]) { + { "required1", "reqarg1" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "-r", "reqarg1"), + options, + (Entry[]) { + { "required1", "reqarg1" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "string1", + "string2", + "-r", "reqarg1"), + options, + (Entry[]) { + { "required1", "reqarg1" }, + {} + }, + STRV_MAKE("string1", + "string2")); + + test_option_parse_one(STRV_MAKE("arg0", + "--optional1=optarg1"), + options, + (Entry[]) { + { "optional1", "optarg1" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "--optional1", "string1"), + options, + (Entry[]) { + { "optional1", NULL }, + {} + }, + STRV_MAKE("string1")); + + test_option_parse_one(STRV_MAKE("arg0", + "-ooptarg1"), + options, + (Entry[]) { + { "optional1", "optarg1" }, + {} + }, + NULL); + + test_option_parse_one(STRV_MAKE("arg0", + "-o", "string1"), + options, + (Entry[]) { + { "optional1", NULL }, + {} + }, + STRV_MAKE("string1")); + + test_option_parse_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"), + options, + (Entry[]) { + { "help" }, + { "version" }, + { "required1", "reqarg1" }, + { "required2", "reqarg2" }, + { "required1", "reqarg3" }, + { "required2", "reqarg4" }, + { "optional1", NULL }, + { "optional2", NULL, }, + { "optional1", "optarg1" }, + { "optional2", "optarg2" }, + { "help" }, + { "required1", "reqarg5" }, + { "required1", "reqarg6" }, + { "optional1", "optarg3" }, + { "optional1", NULL }, + { "optional1", NULL }, + { "help" }, + { "optional1", NULL }, + { "help" }, + { "help" }, + { "optional1", "optarg4" }, + { "help" }, + { "required1", "reqarg6" }, + {} + }, + STRV_MAKE("string1", + "string2", + "string3", + "string4", + "string5", + "string6", + "string7", + "--help", + "--required1", + "--optional1")); +} + +DEFINE_TEST_MAIN(LOG_DEBUG); From d03089ff4f445437022cf312d4062e6a9a98dce2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 11:38:29 +0100 Subject: [PATCH 04/15] verbs: add a section to list verbs similarly to options --- src/shared/verbs.c | 61 +++++++++++++++++++++++++++++----- src/shared/verbs.h | 50 ++++++++++++++++++++++++++-- src/systemctl/systemctl-main.c | 2 +- 3 files changed, 101 insertions(+), 12 deletions(-) diff --git a/src/shared/verbs.c b/src/shared/verbs.c index 8c174a6c88..bf2d440bb0 100644 --- a/src/shared/verbs.c +++ b/src/shared/verbs.c @@ -3,6 +3,7 @@ #include #include "env-util.h" +#include "format-table.h" #include "log.h" #include "string-util.h" #include "strv.h" @@ -57,33 +58,34 @@ bool should_bypass(const char *env_prefix) { return true; } -const Verb* verbs_find_verb(const char *name, const Verb verbs[]) { +const Verb* verbs_find_verb(const char *name, const Verb verbs[], const Verb verbs_end[]) { assert(verbs); - for (size_t i = 0; verbs[i].dispatch; i++) - if (name ? streq(name, verbs[i].verb) : FLAGS_SET(verbs[i].flags, VERB_DEFAULT)) - return verbs + i; + for (const Verb *verb = verbs; verb < verbs_end; verb++) + if (name ? streq(name, verb->verb) : FLAGS_SET(verb->flags, VERB_DEFAULT)) + return verb; /* At the end of the list? */ return NULL; } -int dispatch_verb_with_args(char **args, const Verb verbs[], void *userdata) { +int _dispatch_verb_with_args(char **args, const Verb verbs[], const Verb verbs_end[], void *userdata) { int r; assert(verbs); + assert(verbs_end > verbs); assert(verbs[0].dispatch); assert(verbs[0].verb); const char *name = args ? args[0] : NULL; size_t left = strv_length(args); - const Verb *verb = verbs_find_verb(name, verbs); + const Verb *verb = verbs_find_verb(name, verbs, verbs_end); if (!verb) { _cleanup_strv_free_ char **verb_strv = NULL; - for (size_t i = 0; verbs[i].dispatch; i++) { - r = strv_extend(&verb_strv, verbs[i].verb); + for (verb = verbs; verb < verbs_end; verb++) { + r = strv_extend(&verb_strv, verb->verb); if (r < 0) return log_oom(); } @@ -139,5 +141,46 @@ int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata) { assert(argv); assert(argc >= optind); - return dispatch_verb_with_args(strv_skip(argv, optind), verbs, userdata); + size_t n = 0; + while (verbs[n].dispatch) + n++; + + return _dispatch_verb_with_args(strv_skip(argv, optind), verbs, verbs + n, userdata); +} + +int _verbs_get_help_table(const Verb verbs[], const Verb verbs_end[], Table **ret) { + int r; + + assert(ret); + + _cleanup_(table_unrefp) Table *table = table_new("verb", "help"); + if (!table) + return log_oom(); + + for (const Verb *verb = verbs; verb < verbs_end; verb++) { + assert(verb->dispatch); + + /* We indent the option string by two spaces. We could set the minimum cell width and + * right-align for a similar result, but that'd be more work. This is only used for + * display. */ + r = table_add_cell_stringf(table, NULL, " %s%s%s", + verb->verb, + verb->argspec ? " " : "", + strempty(verb->argspec)); + if (r < 0) + return table_log_add_error(r); + + const char *help = verb->help ?: "FIXME"; + _cleanup_strv_free_ char **s = strv_split(help, /* separators= */ NULL); + if (!s) + return log_oom(); + + r = table_add_many(table, TABLE_STRV_WRAPPED, s); + if (r < 0) + return table_log_add_error(r); + } + + table_set_header(table, false); + *ret = TAKE_PTR(table); + return 0; } diff --git a/src/shared/verbs.h b/src/shared/verbs.h index febb8ccfc2..062e49466f 100644 --- a/src/shared/verbs.h +++ b/src/shared/verbs.h @@ -16,12 +16,58 @@ typedef struct { VerbFlags flags; int (* const dispatch)(int argc, char *argv[], uintptr_t data, void *userdata); uintptr_t data; + const char *argspec; + const char *help; } Verb; +#define VERB_FULL(d, v, a, amin, amax, f, dat, h) \ + static int d(int, char**, uintptr_t, void*); \ + _section_("SYSTEMD_VERBS") \ + _alignptr_ \ + _used_ \ + _retain_ \ + _variable_no_sanitize_address_ \ + static const Verb CONCATENATE(d, _data) = { \ + .verb = v, \ + .min_args = amin, \ + .max_args = amax, \ + .flags = f, \ + .dispatch = d, \ + .data = dat, \ + .argspec = a, \ + .help = h, \ + } + +/* The same as VERB_FULL, but without the data argument */ +#define VERB(d, v, a, amin, amax, f, h) \ + VERB_FULL(d, v, a, amin, amax, f, /* dat= */ 0, h) + +/* Simplified VERB for parameters that take no argument */ +#define VERB_NOARG(d, v, h) \ + VERB(d, v, /* a= */ NULL, /* amin= */ VERB_ANY, /* amax= */ 1, /* f= */ 0, h) + +/* This is magically mapped to the beginning and end of the section */ +extern const Verb __start_SYSTEMD_VERBS[]; +extern const Verb __stop_SYSTEMD_VERBS[]; + bool running_in_chroot_or_offline(void); bool should_bypass(const char *env_prefix); -const Verb* verbs_find_verb(const char *name, const Verb verbs[]); -int dispatch_verb_with_args(char **args, const Verb verbs[], void *userdata); +const Verb* verbs_find_verb(const char *name, const Verb verbs[], const Verb verbs_end[]); + +int _dispatch_verb_with_args(char **args, const Verb verbs[], const Verb verbs_end[], void *userdata); +#define dispatch_verb_with_args(args, userdata) \ + _dispatch_verb_with_args(args, ALIGN_PTR(__start_SYSTEMD_VERBS), __stop_SYSTEMD_VERBS, userdata) + int dispatch_verb(int argc, char *argv[], const Verb verbs[], void *userdata); + +int _verbs_get_help_table(const Verb verbs[], const Verb verbs_end[], Table **ret); +#define verbs_get_help_table(ret) \ + _verbs_get_help_table(ALIGN_PTR(__start_SYSTEMD_VERBS), __stop_SYSTEMD_VERBS, ret) + +#define VERB_COMMON_HELP(impl) \ + VERB(verb_help, "help", NULL, VERB_ANY, VERB_ANY, 0, "Show this help"); \ + static int verb_help(int argc, char **argv, uintptr_t data, void *userdata) { \ + return impl(); \ + } diff --git a/src/systemctl/systemctl-main.c b/src/systemctl/systemctl-main.c index 565b3a8878..4d1830071f 100644 --- a/src/systemctl/systemctl-main.c +++ b/src/systemctl/systemctl-main.c @@ -135,7 +135,7 @@ static int systemctl_main(int argc, char *argv[]) { {} }; - const Verb *verb = verbs_find_verb(argv[optind], verbs); + const Verb *verb = verbs_find_verb(argv[optind], verbs, verbs + ELEMENTSOF(verbs) - 1); if (verb && (verb->flags & VERB_ONLINE_ONLY) && arg_root) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Verb '%s' cannot be used with --root= or --image=.", From 4bf73e6980fb8f440fbf38253041ca08ff2e0e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Mar 2026 17:45:17 +0100 Subject: [PATCH 05/15] test-options: add tests for option macros and flags Add tests for OPTION_STOPS_PARSING, OPTION_GROUP_MARKER, and OPTION_OPTIONAL_ARG flags with manual Option arrays, and a separate test exercising the OPTION, OPTION_LONG, OPTION_SHORT, OPTION_FULL, and OPTION_GROUP macros via FOREACH_OPTION_FULL in a switch statement, as they would be used in real code. Co-developed-by: Claude Opus 4.6 --- src/test/test-options.c | 624 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 622 insertions(+), 2 deletions(-) diff --git a/src/test/test-options.c b/src/test/test-options.c index 56e8dd1d61..6ca29fb849 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -7,6 +7,7 @@ typedef struct Entry { const char *long_code; const char *argument; + char short_code; } Entry; static void test_option_parse_one( @@ -27,7 +28,7 @@ static void test_option_parse_one( for (const Option *o = options; o->short_code != 0 || o->long_code; o++) n_options++; - for (const Entry *e = entries; e && e->long_code; e++) + for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) n_entries++; OptionParser state = {}; @@ -43,7 +44,10 @@ static void test_option_parse_one( strnull(opt->metavar), strnull(arg)); ASSERT_LT(i, n_entries); - ASSERT_TRUE(streq_ptr(opt->long_code, entries[i].long_code)); + if (entries[i].long_code) + ASSERT_TRUE(streq_ptr(opt->long_code, entries[i].long_code)); + if (entries[i].short_code != 0) + ASSERT_EQ(opt->short_code, entries[i].short_code); ASSERT_TRUE(streq_ptr(arg, entries[i].argument)); i++; } @@ -55,6 +59,30 @@ static void test_option_parse_one( ASSERT_STREQ(argv[0], saved_argv0); } +static void test_option_invalid_one( + char **argv, + const Option options[static 1]) { + + _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 argc = strv_length(argv); + + size_t n_options = 0; + for (const Option *o = options; o->short_code != 0 || o->long_code; o++) + n_options++; + + OptionParser state = {}; + const Option *opt; + const char *arg; + + int c = option_parse(options, options + n_options, &state, argc, argv, &opt, &arg); + ASSERT_ERROR(c, EINVAL); +} + TEST(option_parse) { static const Option options[] = { { 1, .short_code = 'h', .long_code = "help" }, @@ -372,4 +400,596 @@ TEST(option_parse) { "--optional1")); } +TEST(option_stops_parsing) { + static const Option options[] = { + { 1, .short_code = 'h', .long_code = "help" }, + { 2, .long_code = "version" }, + { 3, .short_code = 'r', .long_code = "required", .metavar = "ARG" }, + { 4, .long_code = "exec", .flags = OPTION_STOPS_PARSING }, + {} + }; + + /* --exec stops parsing, subsequent --help is positional */ + test_option_parse_one(STRV_MAKE("arg0", + "--exec", + "--help", + "foo"), + options, + (Entry[]) { + { "exec" }, + {} + }, + STRV_MAKE("--help", + "foo")); + + /* Options before --exec are still parsed */ + test_option_parse_one(STRV_MAKE("arg0", + "--help", + "--exec", + "--version", + "bar"), + options, + (Entry[]) { + { "help" }, + { "exec" }, + {} + }, + STRV_MAKE("--version", + "bar")); + + /* --exec with no trailing args */ + test_option_parse_one(STRV_MAKE("arg0", + "--exec"), + options, + (Entry[]) { + { "exec" }, + {} + }, + NULL); + + /* --exec after positional args */ + test_option_parse_one(STRV_MAKE("arg0", + "pos1", + "--exec", + "--help", + "--required", "val"), + options, + (Entry[]) { + { "exec" }, + {} + }, + STRV_MAKE("pos1", + "--help", + "--required", + "val")); + + /* "--" after --exec: "--" is still consumed as end-of-options marker. This is needed for + * backwards compatibility, systemd-dissect implemented this behaviour. But also, it makes + * sense: we're unlikely to ever want to specify "--" as the first argument of whatever + * sequence, but the user may want to specify it for clarity. */ + test_option_parse_one(STRV_MAKE("arg0", + "--exec", + "--", + "--help"), + options, + (Entry[]) { + { "exec" }, + {} + }, + STRV_MAKE("--help")); + + /* "--" before --exec: "--" terminates first, --exec is positional */ + test_option_parse_one(STRV_MAKE("arg0", + "--", + "--exec", + "--help"), + options, + NULL, + STRV_MAKE("--exec", + "--help")); + + /* Multiple options then --exec then more option-like args */ + test_option_parse_one(STRV_MAKE("arg0", + "--help", + "-r", "val1", + "--exec", + "-h", + "--required", "val2"), + options, + (Entry[]) { + { "help" }, + { "required", "val1" }, + { "exec" }, + {} + }, + STRV_MAKE("-h", + "--required", + "val2")); +} + +TEST(option_group_marker) { + static const Option options[] = { + { 1, .short_code = 'h', .long_code = "help" }, + { 2, .long_code = "version" }, + { 0, .long_code = "AdvancedGroup", .flags = OPTION_GROUP_MARKER }, + { 3, .long_code = "debug" }, + { 4, .long_code = "Advance" }, /* prefix match with the group */ + { 5, .long_code = "defilbrilate" }, + {} + }; + + /* Group markers are skipped by the parser — only real options are returned */ + test_option_parse_one(STRV_MAKE("arg0", + "--help", + "--debug"), + options, + (Entry[]) { + { "help" }, + { "debug" }, + {} + }, + NULL); + + /* Check that group marker name is ignored */ + test_option_parse_one(STRV_MAKE("arg0", + "--debug", + "--version"), + options, + (Entry[]) { + { "debug" }, + { "version" }, + {} + }, + NULL); + + /* Verify that the group marker is not mistaken for an option */ + test_option_invalid_one(STRV_MAKE("arg0", + "--AdvancedGroup"), + options); + + /* Verify that the group marker is not mistaken for an option */ + test_option_invalid_one(STRV_MAKE("arg0", + "--AdvancedGroup=2"), + options); + + /* Verify that the group marker is not mistaken for an option, prefix match */ + test_option_invalid_one(STRV_MAKE("arg0", + "--Advanced"), + options); + + /* Check that group marker name is ignored */ + test_option_parse_one(STRV_MAKE("arg0", + "--Advance", + "--Advan"), /* prefix match with unique prefix */ + options, + (Entry[]) { + { "Advance" }, + { "Advance" }, + {} + }, + NULL); + + /* Partial match with multiple candidates */ + test_option_invalid_one(STRV_MAKE("arg0", + "--de"), + options); +} + +TEST(option_optional_arg) { + static const Option options[] = { + { 1, .short_code = 'o', .long_code = "output", .metavar = "FILE", .flags = OPTION_OPTIONAL_ARG }, + { 2, .short_code = 'h', .long_code = "help" }, + {} + }; + + /* Long option with = gets the argument */ + test_option_parse_one(STRV_MAKE("arg0", + "--output=foo.txt"), + options, + (Entry[]) { + { "output", "foo.txt" }, + {} + }, + NULL); + + /* Long option without = does NOT consume the next arg */ + test_option_parse_one(STRV_MAKE("arg0", + "--output", "foo.txt"), + options, + (Entry[]) { + { "output", NULL }, + {} + }, + STRV_MAKE("foo.txt")); + + /* Short option with inline arg */ + test_option_parse_one(STRV_MAKE("arg0", + "-ofoo.txt"), + options, + (Entry[]) { + { "output", "foo.txt" }, + {} + }, + NULL); + + /* Short option without inline arg does NOT consume the next arg */ + test_option_parse_one(STRV_MAKE("arg0", + "-o", "foo.txt"), + options, + (Entry[]) { + { "output", NULL }, + {} + }, + STRV_MAKE("foo.txt")); + + /* Optional arg option at end of argv */ + test_option_parse_one(STRV_MAKE("arg0", + "--output"), + options, + (Entry[]) { + { "output", NULL }, + {} + }, + NULL); + + /* Mixed: optional arg with other options */ + test_option_parse_one(STRV_MAKE("arg0", + "--help", + "--output=bar", + "--help"), + options, + (Entry[]) { + { "help" }, + { "output", "bar" }, + { "help" }, + {} + }, + NULL); + + /* Short combo: -ho (h then o with no arg) */ + test_option_parse_one(STRV_MAKE("arg0", + "-ho", "pos1"), + options, + (Entry[]) { + { "help" }, + { "output", NULL }, + {} + }, + STRV_MAKE("pos1")); + + /* Short combo: -hobar (h then o with inline arg "bar") */ + test_option_parse_one(STRV_MAKE("arg0", + "-hobar"), + options, + (Entry[]) { + { "help" }, + { "output", "bar" }, + {} + }, + NULL); +} + +/* Test the OPTION, OPTION_LONG, OPTION_SHORT, OPTION_FULL, OPTION_GROUP macros + * by using them in a FOREACH_OPTION_FULL switch, as they would be used in real code. */ + +static void test_macros_parse_one( + char **argv, + 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 argc = strv_length(argv); + size_t i = 0, n_entries = 0; + + for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) + n_entries++; + + OptionParser state = {}; + const Option *opt; + const char *arg; + + FOREACH_OPTION_FULL(&state, c, argc, argv, &opt, &arg, ASSERT_TRUE(false)) { + log_debug("%c %s: %s=%s", + opt->short_code != 0 ? opt->short_code : ' ', + opt->long_code ?: "", + strnull(opt->metavar), strnull(arg)); + + ASSERT_LT(i, n_entries); + if (entries[i].long_code) + ASSERT_TRUE(streq_ptr(opt->long_code, entries[i].long_code)); + if (entries[i].short_code != 0) + ASSERT_EQ(opt->short_code, entries[i].short_code); + ASSERT_TRUE(streq_ptr(arg, entries[i].argument)); + i++; + + switch (c) { + + /* OPTION: short + long, no arg */ + OPTION('h', "help", NULL, "Show this help"): + break; + + /* OPTION_LONG: long only, no arg */ + OPTION_LONG("version", NULL, "Show package version"): + break; + + /* OPTION_SHORT: short only, no arg */ + OPTION_SHORT('v', NULL, "Enable verbose mode"): + break; + + /* OPTION: short + long, required arg */ + OPTION('r', "required", "ARG", "Required arg option"): + break; + + /* OPTION_FULL: optional arg */ + OPTION_FULL(OPTION_OPTIONAL_ARG, 'o', "optional", "ARG", "Optional arg option"): + break; + + /* OPTION_FULL: stops parsing */ + OPTION_FULL(OPTION_STOPS_PARSING, 0, "exec", NULL, "Stop parsing after this"): + break; + + /* OPTION_GROUP: group marker (never returned by parser) */ + OPTION_GROUP("Advanced"): + break; + + /* OPTION_LONG: long only, in the "Advanced" group */ + OPTION_LONG("debug", NULL, "Enable debug mode"): + break; + + default: + log_error("Unexpected option id: %d", c); + ASSERT_TRUE(false); + } + } + + ASSERT_EQ(i, n_entries); + + char **args = option_parser_get_args(&state, argc, argv); + ASSERT_TRUE(strv_equal(args, remaining)); + ASSERT_STREQ(argv[0], saved_argv0); +} + +TEST(option_macros) { + /* OPTION: long form */ + test_macros_parse_one(STRV_MAKE("arg0", + "--help"), + (Entry[]) { + { "help" }, + {} + }, + NULL); + + /* OPTION: short form */ + test_macros_parse_one(STRV_MAKE("arg0", + "-h"), + (Entry[]) { + { "help" }, + {} + }, + NULL); + + /* OPTION_LONG: only accessible via long form */ + test_macros_parse_one(STRV_MAKE("arg0", + "--version"), + (Entry[]) { + { "version" }, + {} + }, + NULL); + + /* OPTION_SHORT: only accessible via short form */ + test_macros_parse_one(STRV_MAKE("arg0", + "-v"), + (Entry[]) { + { .short_code = 'v' }, + {} + }, + NULL); + + /* OPTION with required arg: long --required=ARG */ + test_macros_parse_one(STRV_MAKE("arg0", + "--required=val1"), + (Entry[]) { + { "required", "val1" }, + {} + }, + NULL); + + /* OPTION with required arg: long --required ARG */ + test_macros_parse_one(STRV_MAKE("arg0", + "--required", "val1"), + (Entry[]) { + { "required", "val1" }, + {} + }, + NULL); + + /* OPTION with required arg: short -r ARG */ + test_macros_parse_one(STRV_MAKE("arg0", + "-r", "val1"), + (Entry[]) { + { "required", "val1" }, + {} + }, + NULL); + + /* OPTION with required arg: short -rARG */ + test_macros_parse_one(STRV_MAKE("arg0", + "-rval1"), + (Entry[]) { + { "required", "val1" }, + {} + }, + NULL); + + /* OPTION_FULL with OPTION_OPTIONAL_ARG: long with = */ + test_macros_parse_one(STRV_MAKE("arg0", + "--optional=val1"), + (Entry[]) { + { "optional", "val1" }, + {} + }, + NULL); + + /* OPTION_FULL with OPTION_OPTIONAL_ARG: long without = doesn't consume next */ + test_macros_parse_one(STRV_MAKE("arg0", + "--optional", "pos1"), + (Entry[]) { + { "optional", NULL }, + {} + }, + STRV_MAKE("pos1")); + + /* OPTION_FULL with OPTION_OPTIONAL_ARG: short inline */ + test_macros_parse_one(STRV_MAKE("arg0", + "-oval1"), + (Entry[]) { + { "optional", "val1" }, + {} + }, + NULL); + + /* OPTION_FULL with OPTION_OPTIONAL_ARG: short without inline */ + test_macros_parse_one(STRV_MAKE("arg0", + "-o", "pos1"), + (Entry[]) { + { "optional", NULL }, + {} + }, + STRV_MAKE("pos1")); + + /* OPTION_FULL with OPTION_STOPS_PARSING: stops further option parsing */ + test_macros_parse_one(STRV_MAKE("arg0", + "--exec", + "--help", + "--version"), + (Entry[]) { + { "exec" }, + {} + }, + STRV_MAKE("--help", + "--version")); + + /* OPTION_STOPS_PARSING: options before are still parsed */ + test_macros_parse_one(STRV_MAKE("arg0", + "--help", + "--exec", + "-h", + "--debug"), + (Entry[]) { + { "help" }, + { "exec" }, + {} + }, + STRV_MAKE("-h", + "--debug")); + + /* OPTION_STOPS_PARSING with "--": "--" after exec is still consumed */ + test_macros_parse_one(STRV_MAKE("arg0", + "--exec", + "--", + "--help"), + (Entry[]) { + { "exec" }, + {} + }, + STRV_MAKE("--help")); + + /* OPTION_STOPS_PARSING with "--": "--" before exec takes precedence */ + test_macros_parse_one(STRV_MAKE("arg0", + "--", + "--exec", + "--help"), + (Entry[]) { + {} + }, + STRV_MAKE("--exec", + "--help")); + + /* OPTION_GROUP: group marker is transparent to parsing, --debug in Advanced group works */ + test_macros_parse_one(STRV_MAKE("arg0", + "--debug"), + (Entry[]) { + { "debug" }, + {} + }, + NULL); + + /* Mixed: all macro types together */ + test_macros_parse_one(STRV_MAKE("arg0", + "pos1", + "-h", + "--version", + "-v", + "--required=rval", + "--optional=oval", + "--debug", + "pos2", + "-o", + "--help"), + (Entry[]) { + { "help" }, + { "version" }, + { .short_code = 'v' }, + { "required", "rval" }, + { "optional", "oval" }, + { "debug" }, + { "optional", NULL }, + { "help" }, + {} + }, + STRV_MAKE("pos1", + "pos2")); + + /* Short option combos with macros: -hv (help + verbose) */ + test_macros_parse_one(STRV_MAKE("arg0", + "-hv"), + (Entry[]) { + { "help" }, + { .short_code = 'v' }, + {} + }, + NULL); + + /* Short option combo with required arg: -hrval (help + required with arg "val") */ + test_macros_parse_one(STRV_MAKE("arg0", + "-hrval"), + (Entry[]) { + { "help" }, + { "required", "val" }, + {} + }, + NULL); + + /* Short option combo with optional arg: -hoval (help + optional with arg "val") */ + test_macros_parse_one(STRV_MAKE("arg0", + "-hoval"), + (Entry[]) { + { "help" }, + { "optional", "val" }, + {} + }, + NULL); + + /* OPTION_STOPS_PARSING then "--": "--" is still consumed after exec */ + test_macros_parse_one(STRV_MAKE("arg0", + "--help", + "--exec", + "--version", + "--", + "-h"), + (Entry[]) { + { "help" }, + { "exec" }, + {} + }, + STRV_MAKE("--version", + "-h")); +} + DEFINE_TEST_MAIN(LOG_DEBUG); From e13b7872e047b15c0cf3092f1317ed019d17c990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 19 Mar 2026 18:13:41 +0100 Subject: [PATCH 06/15] options: only consume "--" immediately after an option that stops parsing The behaviour that was implemented in systemd-dissect was that both '--exec -- cmd' and '--exec cmd' result in 'cmd' as the command, and '--' anywhere later is as a positional argument, so nesting is possible, e.g.: --exec -- cmd --opt -- another-cmd --another-opt This is not obvious, so add some tests for this and keep it as a separate commit. --- src/shared/options.c | 6 ++++-- src/test/test-options.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/shared/options.c b/src/shared/options.c index 09b677f813..903ed62440 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -112,8 +112,10 @@ int option_parse( return 0; } - if (!state->parsing_stopped && - argv[state->optind][0] == '-' && + if (state->parsing_stopped) + return 0; + + if (argv[state->optind][0] == '-' && argv[state->optind][1] != '\0') /* Looks like we found an option parameter */ break; diff --git a/src/test/test-options.c b/src/test/test-options.c index 6ca29fb849..ab3a429369 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -977,6 +977,21 @@ TEST(option_macros) { NULL); /* OPTION_STOPS_PARSING then "--": "--" is still consumed after exec */ + test_macros_parse_one(STRV_MAKE("arg0", + "--help", + "--exec", + "--", + "--version", + "-h"), + (Entry[]) { + { "help" }, + { "exec" }, + {} + }, + STRV_MAKE("--version", + "-h")); + + /* OPTION_STOPS_PARSING then later "--": "--" is not consumed */ test_macros_parse_one(STRV_MAKE("arg0", "--help", "--exec", @@ -989,6 +1004,24 @@ TEST(option_macros) { {} }, STRV_MAKE("--version", + "--", + "-h")); + + /* OPTION_STOPS_PARSING then "--" twice: second "--" is not consumed */ + test_macros_parse_one(STRV_MAKE("arg0", + "--help", + "--exec", + "--", + "--", + "--version", + "-h"), + (Entry[]) { + { "help" }, + { "exec" }, + {} + }, + STRV_MAKE("--", + "--version", "-h")); } From cefec0b67d93eb291cbd6210731ae9af84715aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 18 Mar 2026 17:38:47 +0100 Subject: [PATCH 07/15] id128: use the new option parser --version was not documented. Fixup for 0d1d512f7f42071595f0c950f911f3557fda09ea. --- src/id128/id128.c | 144 +++++++++++++++++----------------------------- 1 file changed, 53 insertions(+), 91 deletions(-) diff --git a/src/id128/id128.c b/src/id128/id128.c index fecfeeabab..23a028b28c 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include "alloc-util.h" @@ -11,6 +10,7 @@ #include "id128-util.h" #include "log.h" #include "main-func.h" +#include "options.h" #include "parse-argument.h" #include "pretty-print.h" #include "string-util.h" @@ -24,10 +24,12 @@ static PagerFlags arg_pager_flags = 0; static bool arg_legend = true; static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; +VERB_NOARG(verb_new, "new", "Generate a new ID"); static int verb_new(int argc, char *argv[], uintptr_t _data, void *userdata) { return id128_print_new(arg_mode); } +VERB_NOARG(verb_machine_id, "machine-id", "Print the ID of current machine"); static int verb_machine_id(int argc, char *argv[], uintptr_t _data, void *userdata) { sd_id128_t id; int r; @@ -43,6 +45,7 @@ static int verb_machine_id(int argc, char *argv[], uintptr_t _data, void *userda return id128_pretty_print(id, arg_mode); } +VERB_NOARG(verb_boot_id, "boot-id", "Print the ID of current boot"); static int verb_boot_id(int argc, char *argv[], uintptr_t _data, void *userdata) { sd_id128_t id; int r; @@ -58,6 +61,7 @@ static int verb_boot_id(int argc, char *argv[], uintptr_t _data, void *userdata) return id128_pretty_print(id, arg_mode); } +VERB_NOARG(verb_invocation_id, "invocation-id", "Print the ID of current invocation"); static int verb_invocation_id(int argc, char *argv[], uintptr_t _data, void *userdata) { sd_id128_t id; int r; @@ -73,6 +77,7 @@ static int verb_invocation_id(int argc, char *argv[], uintptr_t _data, void *use return id128_pretty_print(id, arg_mode); } +VERB_NOARG(verb_var_uuid, "var-partition-uuid", "Print the UUID for the /var/ partition"); static int verb_var_uuid(int argc, char *argv[], uintptr_t _data, void *userdata) { sd_id128_t id; int r; @@ -130,6 +135,7 @@ static int show_one(Table **table, const char *name, sd_id128_t uuid, bool first arg_mode == ID128_PRINT_ID128 ? TABLE_ID128 : TABLE_UUID, uuid); } +VERB(verb_show, "show", "[NAME|UUID]", VERB_ANY, VERB_ANY, 0, "Print one or more UUIDs"); static int verb_show(int argc, char *argv[], uintptr_t _data, void *userdata) { _cleanup_(table_unrefp) Table *table = NULL; int r; @@ -186,158 +192,114 @@ static int verb_show(int argc, char *argv[], uintptr_t _data, void *userdata) { static int help(void) { _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL, *verbs = NULL; int r; r = terminal_urlify_man("systemd-id128", "1", &link); if (r < 0) return log_oom(); + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + r = verbs_get_help_table(&verbs); + if (r < 0) + return r; + printf("%s [OPTIONS...] COMMAND\n\n" "%sGenerate and print 128-bit identifiers.%s\n" - "\nCommands:\n" - " new Generate a new ID\n" - " machine-id Print the ID of current machine\n" - " boot-id Print the ID of current boot\n" - " invocation-id Print the ID of current invocation\n" - " var-partition-uuid Print the UUID for the /var/ partition\n" - " show [NAME|UUID] Print one or more UUIDs\n" - " help Show this help\n" - "\nOptions:\n" - " -h --help Show this help\n" - " --no-pager Do not pipe output into a pager\n" - " --no-legend Do not show the headers and footers\n" - " --json=FORMAT Output inspection data in JSON (takes one of\n" - " pretty, short, off)\n" - " -j Equivalent to --json=pretty (on TTY) or\n" - " --json=short (otherwise)\n" - " -p --pretty Generate samples of program code\n" - " -P --value Only print the value\n" - " -a --app-specific=ID Generate app-specific IDs\n" - " -u --uuid Output in UUID format\n" - "\nSee the %s for details.\n", + "\nCommands:\n", program_invocation_short_name, ansi_highlight(), - ansi_normal(), - link); + ansi_normal()); + table_print(verbs, stdout); + printf("\nOptions:\n"); + table_print(options, stdout); + + printf("\nSee the %s for details.\n", link); return 0; } -static int verb_help(int argc, char *argv[], uintptr_t _data, void *userdata) { - return help(); -} +VERB_COMMON_HELP(help); -static int parse_argv(int argc, char *argv[]) { - enum { - ARG_VERSION = 0x100, - ARG_NO_PAGER, - ARG_NO_LEGEND, - ARG_JSON, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, - { "json", required_argument, NULL, ARG_JSON }, - { "pretty", no_argument, NULL, 'p' }, - { "value", no_argument, NULL, 'P' }, - { "app-specific", required_argument, NULL, 'a' }, - { "uuid", no_argument, NULL, 'u' }, - {}, - }; - - int c, r; +static int parse_argv(int argc, char *argv[], char ***ret_args) { + int r; assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hpa:uPj", options, NULL)) >= 0) - switch (c) { + OptionParser state = {}; + const char *arg; - case 'h': + FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + switch (c) { + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_NO_LEGEND: + OPTION_COMMON_NO_LEGEND: arg_legend = false; break; - case 'j': + OPTION_LONG("json", "FORMAT", + "Output inspection data in JSON (takes one of pretty, short, off)"): + r = parse_json_argument(arg, &arg_json_format_flags); + if (r <= 0) + return r; + break; + + OPTION_SHORT('j', NULL, + "Equivalent to --json=pretty (on TTY) or --json=short (otherwise)"): arg_json_format_flags = SD_JSON_FORMAT_PRETTY_AUTO|SD_JSON_FORMAT_COLOR_AUTO; break; - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); - if (r <= 0) - return r; - - break; - case 'p': + OPTION('p', "pretty", NULL, "Generate samples of program code"): arg_mode = ID128_PRINT_PRETTY; arg_value = false; break; - case 'P': + OPTION('P', "value", NULL, "Only print the value"): arg_value = true; if (arg_mode == ID128_PRINT_PRETTY) arg_mode = ID128_PRINT_ID128; break; - case 'a': - r = id128_from_string_nonzero(optarg, &arg_app); + OPTION('a', "app-specific", "ID", "Generate app-specific IDs"): + r = id128_from_string_nonzero(arg, &arg_app); if (r == -ENXIO) return log_error_errno(r, "Application ID cannot be all zeros."); if (r < 0) - return log_error_errno(r, "Failed to parse \"%s\" as application-ID: %m", optarg); + return log_error_errno(r, "Failed to parse \"%s\" as application ID: %m", arg); break; - case 'u': + OPTION('u', "uuid", NULL, "Output in UUID format"): arg_mode = ID128_PRINT_UUID; break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } + *ret_args = option_parser_get_args(&state, argc, argv); return 1; } -static int id128_main(int argc, char *argv[]) { - static const Verb verbs[] = { - { "new", VERB_ANY, 1, 0, verb_new }, - { "machine-id", VERB_ANY, 1, 0, verb_machine_id }, - { "boot-id", VERB_ANY, 1, 0, verb_boot_id }, - { "invocation-id", VERB_ANY, 1, 0, verb_invocation_id }, - { "var-partition-uuid", VERB_ANY, 1, 0, verb_var_uuid }, - { "show", VERB_ANY, VERB_ANY, 0, verb_show }, - { "help", VERB_ANY, VERB_ANY, 0, verb_help }, - {} - }; - - return dispatch_verb(argc, argv, verbs, NULL); -} - static int run(int argc, char *argv[]) { int r; log_setup(); - r = parse_argv(argc, argv); + char **args = NULL; /* unnecessary initialization to appease gcc <= 13 */ + r = parse_argv(argc, argv, &args); if (r <= 0) return r; - return id128_main(argc, argv); + return dispatch_verb_with_args(args, NULL); } DEFINE_MAIN_FUNCTION(run); From 34fba9e561d78307e7452303dda33a1fd2b36974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 11:35:38 +0100 Subject: [PATCH 08/15] dissect: use the new option parser Some cosmetic differences in descriptions are made. --quiet was miscategorized as a command (172fadda65a788c6128e5c93e0c530244c6a42e2), --usr-hash and --usr-hash-sig were not listed (10b8d65f3f37f169fdc3999ff86e5fdbdbd0e3a5). --- src/dissect/dissect.c | 610 ++++++++++++++++-------------------------- 1 file changed, 226 insertions(+), 384 deletions(-) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index aafbd872ae..8ede2f61a9 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -42,6 +42,7 @@ #include "mountpoint-util.h" #include "namespace-util.h" #include "nsresource.h" +#include "options.h" #include "parse-argument.h" #include "parse-util.h" #include "path-util.h" @@ -122,6 +123,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_image_filter, image_filter_freep); static int help(void) { _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL, *commands = NULL; int r; pager_open(arg_pager_flags); @@ -130,6 +132,14 @@ static int help(void) { if (r < 0) return log_oom(); + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + r = option_parser_get_help_table_group("Commands", &commands); + if (r < 0) + return r; + printf("%1$s [OPTIONS...] IMAGE\n" "%1$s [OPTIONS...] --mount IMAGE PATH\n" "%1$s [OPTIONS...] --umount PATH\n" @@ -144,112 +154,24 @@ static int help(void) { "%1$s [OPTIONS...] --discover\n" "%1$s [OPTIONS...] --validate IMAGE\n" "%1$s [OPTIONS...] --shift IMAGE UIDBASE\n" - "\n%5$sDissect a Discoverable Disk Image (DDI).%6$s\n\n" - "%3$sOptions:%4$s\n" - " --no-pager Do not pipe output into a pager\n" - " --no-legend Do not show the headers and footers\n" - " -r --read-only Mount read-only\n" - " --fsck=BOOL Run fsck before mounting\n" - " --growfs=BOOL Grow file system to partition size, if marked\n" - " --mkdir Make mount directory before mounting, if missing\n" - " --rmdir Remove mount directory after unmounting\n" - " --discard=MODE Choose 'discard' mode (disabled, loop, all, crypto)\n" - " --in-memory Copy image into memory\n" - " --root-hash=HASH Specify root hash for verity\n" - " --root-hash-sig=SIG Specify pkcs7 signature of root hash for verity\n" - " as a DER encoded PKCS7, either as a path to a file\n" - " or as an ASCII base64 encoded string prefixed by\n" - " 'base64:'\n" - " --verity-data=PATH Specify data file with hash tree for verity if it is\n" - " not embedded in IMAGE\n" - " --image-policy=POLICY\n" - " Specify image dissection policy\n" - " --image-filter=FILTER\n" - " Specify image dissection filter\n" - " --json=pretty|short|off\n" - " Generate JSON output\n" - " --loop-ref=NAME Set reference string for loopback device\n" - " --loop-ref-auto Derive reference string from image file name\n" - " --mtree-hash=BOOL Whether to include SHA256 hash in the mtree output\n" - " --copy-ownership=BOOL\n" - " Whether to copy ownership when copying files\n" - " --user Discover user images\n" - " --system Discover system images\n" - " --all Show hidden images too\n" - "\n%3$sCommands:%4$s\n" - " -h --help Show this help\n" - " --version Show package version\n" - " -m --mount Mount the image to the specified directory\n" - " -M Shortcut for --mount --mkdir\n" - " -u --umount Unmount the image from the specified directory\n" - " -U Shortcut for --umount --rmdir\n" - " --attach Attach the disk image to a loopback block device\n" - " --detach Detach a loopback block device again\n" - " -l --list List all the files and directories of the specified\n" - " OS image\n" - " --mtree Show BSD mtree manifest of OS image\n" - " --with Mount, run command, unmount\n" - " -x --copy-from Copy files from image to host\n" - " -a --copy-to Copy files from host to image\n" - " --make-archive Convert the DDI to an archive file\n" - " --discover Discover DDIs in well known directories\n" - " --validate Validate image and image policy\n" - " --shift Shift UID range to selected base\n" - " -q --quiet Suppress output of chosen loopback block device\n" - "\nSee the %2$s for details.\n", + "\n%2$sDissect a Discoverable Disk Image (DDI).%3$s\n" + "\n%4$sOptions:%5$s\n", program_invocation_short_name, - link, - ansi_underline(), - ansi_normal(), ansi_highlight(), + ansi_normal(), + ansi_underline(), ansi_normal()); + table_print(options, stdout); + + printf("\n%sCommands:%s\n", ansi_underline(), ansi_normal()); + + table_print(commands, stdout); + + printf("\nSee the %s for details.\n", link); return 0; } -static int patch_argv(int *argc, char ***argv, char ***buf) { - _cleanup_free_ char **l = NULL; - char **e; - - assert(argc); - assert(*argc >= 0); - assert(argv); - assert(*argv); - assert(buf); - - /* Ugly hack: if --with is included in command line, also insert "--" immediately after it, to make - * getopt_long() stop processing switches */ - - for (e = *argv + 1; e < *argv + *argc; e++) { - assert(*e); - - if (streq(*e, "--with")) - break; - } - - if (e >= *argv + *argc || streq_ptr(e[1], "--")) { - /* No --with used? Or already followed by "--"? Then don't do anything */ - *buf = NULL; - return 0; - } - - /* Insert the extra "--" right after the --with */ - l = new(char*, *argc + 2); - if (!l) - return log_oom(); - - size_t idx = e - *argv + 1; - memcpy(l, *argv, sizeof(char*) * idx); /* copy everything up to and including the --with */ - l[idx] = (char*) "--"; /* insert "--" */ - memcpy(l + idx + 1, e + 1, sizeof(char*) * (*argc - idx + 1)); /* copy the rest, including trailing NULL entry */ - - (*argc)++; - (*argv) = l; - - *buf = TAKE_PTR(l); - return 1; -} - static int parse_image_path_argument(const char *path, char **ret_root, char **ret_image) { _cleanup_free_ char *p = NULL; struct stat st; @@ -276,190 +198,51 @@ static int parse_image_path_argument(const char *path, char **ret_root, char **r } static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_VERSION = 0x100, - ARG_NO_PAGER, - ARG_NO_LEGEND, - ARG_WITH, - ARG_DISCARD, - ARG_FSCK, - ARG_GROWFS, - ARG_ROOT_HASH, - ARG_ROOT_HASH_SIG, - ARG_USR_HASH, - ARG_USR_HASH_SIG, - ARG_VERITY_DATA, - ARG_MKDIR, - ARG_RMDIR, - ARG_IN_MEMORY, - ARG_JSON, - ARG_MTREE, - ARG_DISCOVER, - ARG_ATTACH, - ARG_DETACH, - ARG_LOOP_REF, - ARG_LOOP_REF_AUTO, - ARG_IMAGE_POLICY, - ARG_VALIDATE, - ARG_MTREE_HASH, - ARG_MAKE_ARCHIVE, - ARG_SHIFT, - ARG_SYSTEM, - ARG_USER, - ARG_ALL, - ARG_IMAGE_FILTER, - ARG_COPY_OWNERSHIP, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, - { "mount", no_argument, NULL, 'm' }, - { "umount", no_argument, NULL, 'u' }, - { "attach", no_argument, NULL, ARG_ATTACH }, - { "detach", no_argument, NULL, ARG_DETACH }, - { "with", no_argument, NULL, ARG_WITH }, - { "read-only", no_argument, NULL, 'r' }, - { "discard", required_argument, NULL, ARG_DISCARD }, - { "fsck", required_argument, NULL, ARG_FSCK }, - { "growfs", required_argument, NULL, ARG_GROWFS }, - { "root-hash", required_argument, NULL, ARG_ROOT_HASH }, - { "root-hash-sig", required_argument, NULL, ARG_ROOT_HASH_SIG }, - { "usr-hash", required_argument, NULL, ARG_USR_HASH }, - { "usr-hash-sig", required_argument, NULL, ARG_USR_HASH_SIG }, - { "verity-data", required_argument, NULL, ARG_VERITY_DATA }, - { "mkdir", no_argument, NULL, ARG_MKDIR }, - { "rmdir", no_argument, NULL, ARG_RMDIR }, - { "in-memory", no_argument, NULL, ARG_IN_MEMORY }, - { "list", no_argument, NULL, 'l' }, - { "mtree", no_argument, NULL, ARG_MTREE }, - { "copy-from", no_argument, NULL, 'x' }, - { "copy-to", no_argument, NULL, 'a' }, - { "json", required_argument, NULL, ARG_JSON }, - { "discover", no_argument, NULL, ARG_DISCOVER }, - { "loop-ref", required_argument, NULL, ARG_LOOP_REF }, - { "loop-ref-auto", no_argument, NULL, ARG_LOOP_REF_AUTO }, - { "image-policy", required_argument, NULL, ARG_IMAGE_POLICY }, - { "validate", no_argument, NULL, ARG_VALIDATE }, - { "mtree-hash", required_argument, NULL, ARG_MTREE_HASH }, - { "make-archive", no_argument, NULL, ARG_MAKE_ARCHIVE }, - { "shift", no_argument, NULL, ARG_SHIFT }, - { "system", no_argument, NULL, ARG_SYSTEM }, - { "user", no_argument, NULL, ARG_USER }, - { "all", no_argument, NULL, ARG_ALL }, - { "quiet", no_argument, NULL, 'q' }, - { "image-filter", required_argument, NULL, ARG_IMAGE_FILTER }, - { "copy-ownership", required_argument, NULL, ARG_COPY_OWNERSHIP }, - {} - }; - - _cleanup_free_ char **buf = NULL; /* we use free(), not strv_free() here, as we don't copy the strings here */ bool system_scope_requested = false, user_scope_requested = false; - int c, r; + int r; assert(argc >= 0); assert(argv); - r = patch_argv(&argc, &argv, &buf); - if (r < 0) - return r; - - while ((c = getopt_long(argc, argv, "hmurMUlxaq", options, NULL)) >= 0) { + OptionParser state = {}; + const Option *current; + const char *arg; + FOREACH_OPTION_FULL(&state, c, argc, argv, ¤t, &arg, /* on_error= */ return c) switch (c) { - case 'h': - return help(); - - case ARG_VERSION: - return version(); - - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_NO_LEGEND: + OPTION_COMMON_NO_LEGEND: arg_legend = false; break; - case 'm': - arg_action = ACTION_MOUNT; - break; - - case ARG_MKDIR: + OPTION_LONG("mkdir", NULL, "Make mount directory before mounting, if missing"): arg_flags |= DISSECT_IMAGE_MKDIR; break; - case 'M': - /* Shortcut combination of the above two */ - arg_action = ACTION_MOUNT; - arg_flags |= DISSECT_IMAGE_MKDIR; - break; - - case 'u': - arg_action = ACTION_UMOUNT; - break; - - case ARG_RMDIR: + OPTION_LONG("rmdir", NULL, "Remove mount directory after unmounting"): arg_rmdir = true; break; - case 'U': - /* Shortcut combination of the above two */ - arg_action = ACTION_UMOUNT; - arg_rmdir = true; - break; - - case ARG_ATTACH: - arg_action = ACTION_ATTACH; - break; - - case ARG_DETACH: - arg_action = ACTION_DETACH; - break; - - case 'l': - arg_action = ACTION_LIST; + OPTION('r', "read-only", NULL, "Mount read-only"): arg_flags |= DISSECT_IMAGE_READ_ONLY; break; - case ARG_MTREE: - arg_action = ACTION_MTREE; - arg_flags |= DISSECT_IMAGE_READ_ONLY; - break; - - case ARG_WITH: - arg_action = ACTION_WITH; - break; - - case 'x': - arg_action = ACTION_COPY_FROM; - arg_flags |= DISSECT_IMAGE_READ_ONLY; - break; - - case 'a': - arg_action = ACTION_COPY_TO; - break; - - case 'r': - arg_flags |= DISSECT_IMAGE_READ_ONLY; - break; - - case ARG_DISCARD: { + OPTION_LONG("discard", "MODE", "Choose discard mode (disabled, loop, all, crypto)"): { DissectImageFlags flags; - if (streq(optarg, "disabled")) + if (streq(arg, "disabled")) flags = 0; - else if (streq(optarg, "loop")) + else if (streq(arg, "loop")) flags = DISSECT_IMAGE_DISCARD_ON_LOOP; - else if (streq(optarg, "all")) + else if (streq(arg, "all")) flags = DISSECT_IMAGE_DISCARD_ON_LOOP | DISSECT_IMAGE_DISCARD; - else if (streq(optarg, "crypt")) + else if (streq(arg, "crypt")) flags = DISSECT_IMAGE_DISCARD_ANY; - else if (streq(optarg, "list")) { + else if (streq(arg, "list")) { puts("disabled\n" "all\n" "crypt\n" @@ -467,32 +250,32 @@ static int parse_argv(int argc, char *argv[]) { return 0; } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Unknown --discard= parameter: %s", - optarg); + "Unknown --discard= parameter: %s", arg); arg_flags = (arg_flags & ~DISSECT_IMAGE_DISCARD_ANY) | flags; break; } - case ARG_IN_MEMORY: + OPTION_LONG("in-memory", NULL, "Copy image into memory"): arg_in_memory = true; break; - case ARG_ROOT_HASH: - case ARG_USR_HASH: { + OPTION_LONG("root-hash", "HASH", "Specify root hash for verity"): + OPTION_LONG("usr-hash", "HASH", "Same, but for the usr partition"): { _cleanup_(iovec_done) struct iovec roothash = {}; - PartitionDesignator d = c == ARG_USR_HASH ? PARTITION_USR : PARTITION_ROOT; + PartitionDesignator d = streq(current->long_code, "root-hash") ? PARTITION_ROOT : PARTITION_USR; if (arg_verity_settings.designator >= 0 && arg_verity_settings.designator != d) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot combine --root-hash=/--root-hash-sig= and --usr-hash=/--usr-hash-sig= options."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Cannot combine --root-hash=/--root-hash-sig= and --usr-hash=/--usr-hash-sig= options."); - r = unhexmem(optarg, &roothash.iov_base, &roothash.iov_len); + r = unhexmem(arg, &roothash.iov_base, &roothash.iov_len); if (r < 0) - return log_error_errno(r, "Failed to parse root hash '%s': %m", optarg); + return log_error_errno(r, "Failed to parse root hash '%s': %m", arg); if (roothash.iov_len < sizeof(sd_id128_t)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "Root hash must be at least 128-bit long: %s", optarg); + "Root hash must be at least 128-bit long: %s", arg); iovec_done(&arg_verity_settings.root_hash); arg_verity_settings.root_hash = TAKE_STRUCT(roothash); @@ -500,24 +283,28 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_ROOT_HASH_SIG: - case ARG_USR_HASH_SIG: { - char *value; + OPTION_LONG("root-hash-sig", "SIG", + "Specify signature of root hash for verity as DER-encoded PKCS7, " + "either as a path to a file or as an ASCII base64-encoded string " + "prefixed by 'base64:'"): + OPTION_LONG("usr-hash-sig", "SIG", "Same, but for the usr partition"): { + const char *value; _cleanup_(iovec_done) struct iovec sig = {}; - PartitionDesignator d = c == ARG_USR_HASH_SIG ? PARTITION_USR : PARTITION_ROOT; + PartitionDesignator d = streq(current->long_code, "root-hash-sig") ? PARTITION_ROOT : PARTITION_USR; if (arg_verity_settings.designator >= 0 && arg_verity_settings.designator != d) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot combine --root-hash=/--root-hash-sig= and --usr-hash=/--usr-hash-sig= options."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Cannot combine --root-hash=/--root-hash-sig= and --usr-hash=/--usr-hash-sig= options."); - if ((value = startswith(optarg, "base64:"))) { + if ((value = startswith(arg, "base64:"))) { r = unbase64mem(value, &sig.iov_base, &sig.iov_len); if (r < 0) - return log_error_errno(r, "Failed to parse root hash signature '%s': %m", optarg); + return log_error_errno(r, "Failed to parse root hash signature '%s': %m", arg); } else { - r = read_full_file(optarg, (char**) &sig.iov_base, &sig.iov_len); + r = read_full_file(arg, (char**) &sig.iov_base, &sig.iov_len); if (r < 0) - return log_error_errno(r, "Failed to read root hash signature file '%s': %m", optarg); + return log_error_errno(r, "Failed to read root hash signature file '%s': %m", arg); } iovec_done(&arg_verity_settings.root_hash_sig); @@ -526,78 +313,161 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_VERITY_DATA: - r = parse_path_argument(optarg, false, &arg_verity_settings.data_path); + OPTION_LONG("verity-data", "PATH", + "Specify data file with hash tree for verity if it is not embedded in IMAGE"): + r = parse_path_argument(arg, false, &arg_verity_settings.data_path); if (r < 0) return r; break; - case ARG_FSCK: - r = parse_boolean(optarg); + OPTION_LONG("fsck", "BOOL", "Run fsck before mounting"): + r = parse_boolean(arg); if (r < 0) - return log_error_errno(r, "Failed to parse --fsck= parameter: %s", optarg); + return log_error_errno(r, "Failed to parse --fsck= parameter: %s", arg); SET_FLAG(arg_flags, DISSECT_IMAGE_FSCK, r); break; - case ARG_GROWFS: - r = parse_boolean(optarg); + OPTION_LONG("growfs", "BOOL", "Grow file system to partition size, if marked"): + r = parse_boolean(arg); if (r < 0) - return log_error_errno(r, "Failed to parse --growfs= parameter: %s", optarg); + return log_error_errno(r, "Failed to parse --growfs= parameter: %s", arg); SET_FLAG(arg_flags, DISSECT_IMAGE_GROWFS, r); break; - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); + OPTION_COMMON_JSON: + r = parse_json_argument(arg, &arg_json_format_flags); if (r <= 0) return r; - break; - case ARG_DISCOVER: - arg_action = ACTION_DISCOVER; - break; - - case ARG_LOOP_REF: - if (isempty(optarg)) { + OPTION_LONG("loop-ref", "NAME", "Set reference string for loopback device"): + if (isempty(arg)) { arg_loop_ref = mfree(arg_loop_ref); arg_loop_ref_auto = false; break; } - if (strlen(optarg) >= sizeof_field(struct loop_info64, lo_file_name)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Loop device ref string '%s' is too long.", optarg); + if (strlen(arg) >= sizeof_field(struct loop_info64, lo_file_name)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Loop device ref string '%s' is too long.", arg); - r = free_and_strdup_warn(&arg_loop_ref, optarg); + r = free_and_strdup_warn(&arg_loop_ref, arg); if (r < 0) return r; arg_loop_ref_auto = false; break; - case ARG_LOOP_REF_AUTO: + OPTION_LONG("loop-ref-auto", NULL, "Derive reference string from image file name"): arg_loop_ref = mfree(arg_loop_ref); arg_loop_ref_auto = true; break; - case ARG_IMAGE_POLICY: - r = parse_image_policy_argument(optarg, &arg_image_policy); + OPTION_LONG("image-policy", "POLICY", "Specify image dissection policy"): + r = parse_image_policy_argument(arg, &arg_image_policy); if (r < 0) return r; break; - case ARG_VALIDATE: - arg_action = ACTION_VALIDATE; - break; - - case ARG_MTREE_HASH: - r = parse_boolean_argument("--mtree-hash=", optarg, &arg_mtree_hash); + OPTION_LONG("mtree-hash", "BOOL", "Whether to include SHA256 hash in the mtree output"): + r = parse_boolean_argument("--mtree-hash=", arg, &arg_mtree_hash); if (r < 0) return r; break; - case ARG_MAKE_ARCHIVE: + OPTION_LONG("system", NULL, "Discover system images"): + system_scope_requested = true; + break; + + OPTION_LONG("user", NULL, "Discover user images"): + user_scope_requested = true; + break; + + OPTION_LONG("all", NULL, "Show hidden images too"): + arg_all = true; + break; + + OPTION('q', "quiet", NULL, "Suppress output of chosen loopback block device"): + arg_quiet = true; + break; + + OPTION_LONG("image-filter", "FILTER", "Specify image dissection filter"): { + _cleanup_(image_filter_freep) ImageFilter *f = NULL; + r = image_filter_parse(arg, &f); + if (r < 0) + return log_error_errno(r, "Failed to parse image filter expression: %s", arg); + + image_filter_free(arg_image_filter); + arg_image_filter = TAKE_PTR(f); + break; + } + + OPTION_LONG("copy-ownership", "BOOL", "Whether to copy ownership when copying files"): + r = parse_tristate_argument_with_auto("--copy-ownership=", arg, &arg_copy_ownership); + if (r < 0) + return r; + break; + + /************************************ Commands ***************************************/ + OPTION_GROUP("Commands"): + + OPTION_COMMON_HELP: + return help(); + + OPTION_COMMON_VERSION: + return version(); + + OPTION('m', "mount", NULL, "Mount the image to the specified directory"): + arg_action = ACTION_MOUNT; + break; + + OPTION_SHORT('M', NULL, "Shortcut for --mount --mkdir"): + arg_action = ACTION_MOUNT; + arg_flags |= DISSECT_IMAGE_MKDIR; + break; + + OPTION('u', "umount", NULL, "Unmount the image from the specified directory"): + arg_action = ACTION_UMOUNT; + break; + + OPTION_SHORT('U', NULL, "Shortcut for --umount --rmdir"): + arg_action = ACTION_UMOUNT; + arg_rmdir = true; + break; + + OPTION_LONG("attach", NULL, "Attach the disk image to a loopback block device"): + arg_action = ACTION_ATTACH; + break; + + OPTION_LONG("detach", NULL, "Detach a loopback block device again"): + arg_action = ACTION_DETACH; + break; + + OPTION('l', "list", NULL, "List all the files and directories of the specified OS image"): + arg_action = ACTION_LIST; + arg_flags |= DISSECT_IMAGE_READ_ONLY; + break; + + OPTION_LONG("mtree", NULL, "Show BSD mtree manifest of OS image"): + arg_action = ACTION_MTREE; + arg_flags |= DISSECT_IMAGE_READ_ONLY; + break; + + OPTION_FULL(OPTION_STOPS_PARSING, /* sc= */ 0, "with", NULL, "Mount, run command, unmount"): + arg_action = ACTION_WITH; + break; + + OPTION('x', "copy-from", NULL, "Copy files from image to host"): + arg_action = ACTION_COPY_FROM; + arg_flags |= DISSECT_IMAGE_READ_ONLY; + break; + + OPTION('a', "copy-to", NULL, "Copy files from host to image"): + arg_action = ACTION_COPY_TO; + break; + + OPTION_LONG("make-archive", NULL, "Convert the DDI to an archive file"): r = dlopen_libarchive(); if (r < 0) return log_error_errno(r, "Archive support not available (compiled without libarchive, or libarchive not installed?)."); @@ -605,63 +475,33 @@ static int parse_argv(int argc, char *argv[]) { arg_action = ACTION_MAKE_ARCHIVE; break; - case ARG_SHIFT: + OPTION_LONG("discover", NULL, "Discover DDIs in well known directories"): + arg_action = ACTION_DISCOVER; + break; + + OPTION_LONG("validate", NULL, "Validate image and image policy"): + arg_action = ACTION_VALIDATE; + break; + + OPTION_LONG("shift", NULL, "Shift UID range to selected base"): arg_action = ACTION_SHIFT; break; - - case ARG_SYSTEM: - system_scope_requested = true; - break; - - case ARG_USER: - user_scope_requested = true; - break; - - case ARG_ALL: - arg_all = true; - break; - - case 'q': - arg_quiet = true; - break; - - case ARG_IMAGE_FILTER: { - _cleanup_(image_filter_freep) ImageFilter *f = NULL; - r = image_filter_parse(optarg, &f); - if (r < 0) - return log_error_errno(r, "Failed to parse image filter expression: %s", optarg); - - image_filter_free(arg_image_filter); - arg_image_filter = TAKE_PTR(f); - break; } - case ARG_COPY_OWNERSHIP: - r = parse_tristate_argument_with_auto("--copy-ownership=", optarg, &arg_copy_ownership); - if (r < 0) - return r; - break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); - } - } - if (system_scope_requested || user_scope_requested) arg_runtime_scope = system_scope_requested && user_scope_requested ? _RUNTIME_SCOPE_INVALID : system_scope_requested ? RUNTIME_SCOPE_SYSTEM : RUNTIME_SCOPE_USER; + char **args = option_parser_get_args(&state, argc, argv); + switch (arg_action) { case ACTION_DISSECT: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path as only argument."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; @@ -670,15 +510,15 @@ static int parse_argv(int argc, char *argv[]) { break; case ACTION_MOUNT: - if (optind + 2 != argc) + if (strv_length(args) != 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path and mount point path as only arguments."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; - r = parse_path_argument(argv[optind+1], /* suppress_root= */ false, &arg_path); + r = parse_path_argument(args[1], /* suppress_root= */ false, &arg_path); if (r < 0) return r; @@ -686,42 +526,42 @@ static int parse_argv(int argc, char *argv[]) { break; case ACTION_UMOUNT: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected a mount point path as only argument."); - r = parse_path_argument(argv[optind], /* suppress_root= */ false, &arg_path); + r = parse_path_argument(args[0], /* suppress_root= */ false, &arg_path); if (r < 0) return r; break; case ACTION_ATTACH: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path as only argument."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; break; case ACTION_DETACH: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path or loopback device as only argument."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; break; case ACTION_LIST: case ACTION_MTREE: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file or directory path as only argument."); - r = parse_image_path_argument(argv[optind], &arg_root, &arg_image); + r = parse_image_path_argument(args[0], &arg_root, &arg_image); if (r < 0) return r; @@ -729,63 +569,63 @@ static int parse_argv(int argc, char *argv[]) { break; case ACTION_MAKE_ARCHIVE: - if (argc < optind + 1 || argc > optind + 2) + if (!IN_SET(strv_length(args), 1, 2)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file, and an optional target path as only arguments."); - r = parse_image_path_argument(argv[optind], &arg_root, &arg_image); + r = parse_image_path_argument(args[0], &arg_root, &arg_image); if (r < 0) return r; - arg_target = argc > optind + 1 ? empty_or_dash_to_null(argv[optind + 1]) : NULL; + arg_target = empty_or_dash_to_null(args[1]); arg_flags |= DISSECT_IMAGE_READ_ONLY | DISSECT_IMAGE_REQUIRE_ROOT; break; case ACTION_COPY_FROM: - if (argc < optind + 2 || argc > optind + 3) + if (!IN_SET(strv_length(args), 2, 3)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file or directory path, a source path and an optional destination path as only arguments."); - r = parse_image_path_argument(argv[optind], &arg_root, &arg_image); + r = parse_image_path_argument(args[0], &arg_root, &arg_image); if (r < 0) return r; - arg_source = argv[optind + 1]; - arg_target = argc > optind + 2 ? argv[optind + 2] : "-" /* this means stdout */ ; + arg_source = args[1]; + arg_target = args[2] ?: "-"; /* this means stdout */ ; arg_flags |= DISSECT_IMAGE_READ_ONLY | DISSECT_IMAGE_REQUIRE_ROOT; break; case ACTION_COPY_TO: - if (argc < optind + 2 || argc > optind + 3) + if (!IN_SET(strv_length(args), 2, 3)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file or directory path, an optional source path and a destination path as only arguments."); - r = parse_image_path_argument(argv[optind], &arg_root, &arg_image); + r = parse_image_path_argument(args[0], &arg_root, &arg_image); if (r < 0) return r; - if (argc > optind + 2) { - arg_source = argv[optind + 1]; - arg_target = argv[optind + 2]; + if (args[2]) { + arg_source = args[1]; + arg_target = args[2]; } else { arg_source = "-"; /* this means stdin */ - arg_target = argv[optind + 1]; + arg_target = args[1]; } arg_flags |= DISSECT_IMAGE_REQUIRE_ROOT; break; case ACTION_WITH: - if (optind >= argc) + if (strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path and an optional command line."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; - if (argc > optind + 1) { - arg_argv = strv_copy(argv + optind + 1); + if (args[1]) { + arg_argv = strv_copy(args + 1); if (!arg_argv) return log_oom(); } @@ -793,17 +633,17 @@ static int parse_argv(int argc, char *argv[]) { break; case ACTION_DISCOVER: - if (optind != argc) + if (!strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected no argument."); break; case ACTION_VALIDATE: - if (optind + 1 != argc) + if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image file path as only argument."); - r = parse_image_path_argument(argv[optind], NULL, &arg_image); + r = parse_image_path_argument(args[0], NULL, &arg_image); if (r < 0) return r; @@ -812,27 +652,29 @@ static int parse_argv(int argc, char *argv[]) { break; case ACTION_SHIFT: - if (optind + 2 != argc) + if (strv_length(args) != 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Expected an image path and a UID base as only argument."); - r = parse_image_path_argument(argv[optind], &arg_root, &arg_image); + r = parse_image_path_argument(args[0], &arg_root, &arg_image); if (r < 0) return r; - if (streq(argv[optind + 1], "foreign")) + if (streq(args[1], "foreign")) arg_uid_base = FOREIGN_UID_BASE; else { - r = parse_uid(argv[optind + 1], &arg_uid_base); + r = parse_uid(args[1], &arg_uid_base); if (r < 0) - return log_error_errno(r, "Failed to parse UID base: %s", argv[optind + 1]); + return log_error_errno(r, "Failed to parse UID base: %s", args[1]); if ((arg_uid_base & 0xFFFF) != 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Selected UID base not a multiple of 64K: " UID_FMT, arg_uid_base); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Selected UID base not a multiple of 64K: " UID_FMT, arg_uid_base); if (arg_uid_base != 0 && !uid_is_container(arg_uid_base) && !uid_is_foreign(arg_uid_base)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Selected UID range is not in the container range, nor the foreign one, refusing."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Selected UID range is not in the container range, nor the foreign one, refusing."); } arg_flags |= DISSECT_IMAGE_REQUIRE_ROOT; From 08f7a4865e361bfcac3c480152cbbf5e10829340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 21:16:37 +0100 Subject: [PATCH 09/15] dissect: restore compat with clang clang says: src/dissect/dissect.c:292:17: warning: label followed by a declaration is a C23 extension [-Wc23-extensions] Another option would be to raise the C standard to C23. I think that'd make sense, but it's better to do it separately. --- src/dissect/dissect.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 8ede2f61a9..76c3f90c7e 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -260,7 +260,7 @@ static int parse_argv(int argc, char *argv[]) { arg_in_memory = true; break; - OPTION_LONG("root-hash", "HASH", "Specify root hash for verity"): + OPTION_LONG("root-hash", "HASH", "Specify root hash for verity"): {} OPTION_LONG("usr-hash", "HASH", "Same, but for the usr partition"): { _cleanup_(iovec_done) struct iovec roothash = {}; @@ -286,7 +286,7 @@ static int parse_argv(int argc, char *argv[]) { OPTION_LONG("root-hash-sig", "SIG", "Specify signature of root hash for verity as DER-encoded PKCS7, " "either as a path to a file or as an ASCII base64-encoded string " - "prefixed by 'base64:'"): + "prefixed by 'base64:'"): {} OPTION_LONG("usr-hash-sig", "SIG", "Same, but for the usr partition"): { const char *value; _cleanup_(iovec_done) struct iovec sig = {}; @@ -410,7 +410,7 @@ static int parse_argv(int argc, char *argv[]) { break; /************************************ Commands ***************************************/ - OPTION_GROUP("Commands"): + OPTION_GROUP("Commands"): {} OPTION_COMMON_HELP: return help(); From 0e4459eb19eca4dee60513dad6ae49d4f36e83c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 20:20:53 +0100 Subject: [PATCH 10/15] notify: use the new option parser Cosmetic adjustments to one help string. --- src/notify/notify.c | 202 +++++++++++++++++--------------------------- 1 file changed, 77 insertions(+), 125 deletions(-) diff --git a/src/notify/notify.c b/src/notify/notify.c index 6a39147f99..cdca928f90 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include #include @@ -16,10 +15,12 @@ #include "exit-status.h" #include "fd-util.h" #include "fdset.h" +#include "format-table.h" #include "format-util.h" #include "log.h" #include "main-func.h" #include "notify-recv.h" +#include "options.h" #include "parse-util.h" #include "pidref.h" #include "pretty-print.h" @@ -57,40 +58,28 @@ STATIC_DESTRUCTOR_REGISTER(arg_fdname, freep); static int help(void) { _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *options = NULL; int r; r = terminal_urlify_man("systemd-notify", "1", &link); if (r < 0) return log_oom(); - printf("%s [OPTIONS...] [VARIABLE=VALUE...]\n" - "%s [OPTIONS...] --exec [VARIABLE=VALUE...] ; -- CMDLINE...\n" - "%s [OPTIONS...] --fork -- CMDLINE...\n" - "\n%sNotify the init system about service status updates.%s\n\n" - " -h --help Show this help\n" - " --version Show package version\n" - " --ready Inform the service manager about service start-up/reload\n" - " completion\n" - " --reloading Inform the service manager about configuration reloading\n" - " --stopping Inform the service manager about service shutdown\n" - " --pid[=PID] Set main PID of daemon\n" - " --uid=USER Set user to send from\n" - " --status=TEXT Set status text\n" - " --booted Check if the system was booted up with systemd\n" - " --no-block Do not wait until operation finished\n" - " --exec Execute command line separated by ';' once done\n" - " --fd=FD Pass specified file descriptor with along with message\n" - " --fdname=NAME Name to assign to passed file descriptor(s)\n" - " --fork Receive notifications from child rather than sending them\n" - " -q --quiet Do not show PID of child when forking\n" - "\nSee the %s for details.\n", - program_invocation_short_name, - program_invocation_short_name, + r = option_parser_get_help_table(&options); + if (r < 0) + return r; + + printf("%1$s [OPTIONS...] [VARIABLE=VALUE...]\n" + "%1$s [OPTIONS...] --exec [VARIABLE=VALUE...] ; -- CMDLINE...\n" + "%1$s [OPTIONS...] --fork -- CMDLINE...\n" + "\n%2$sNotify the init system about service status updates.%3$s\n\n", program_invocation_short_name, ansi_highlight(), - ansi_normal(), - link); + ansi_normal()); + table_print(options, stdout); + + printf("\nSee the %s for details.\n", link); return 0; } @@ -161,123 +150,87 @@ from_self: return pidref_set_self(ret); } -static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_READY = 0x100, - ARG_RELOADING, - ARG_STOPPING, - ARG_VERSION, - ARG_PID, - ARG_STATUS, - ARG_BOOTED, - ARG_UID, - ARG_NO_BLOCK, - ARG_EXEC, - ARG_FD, - ARG_FDNAME, - ARG_FORK, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "ready", no_argument, NULL, ARG_READY }, - { "reloading", no_argument, NULL, ARG_RELOADING }, - { "stopping", no_argument, NULL, ARG_STOPPING }, - { "pid", optional_argument, NULL, ARG_PID }, - { "status", required_argument, NULL, ARG_STATUS }, - { "booted", no_argument, NULL, ARG_BOOTED }, - { "uid", required_argument, NULL, ARG_UID }, - { "no-block", no_argument, NULL, ARG_NO_BLOCK }, - { "exec", no_argument, NULL, ARG_EXEC }, - { "fd", required_argument, NULL, ARG_FD }, - { "fdname", required_argument, NULL, ARG_FDNAME }, - { "fork", no_argument, NULL, ARG_FORK }, - { "quiet", no_argument, NULL, 'q' }, - {} - }; - +static int parse_argv(int argc, char *argv[], char ***ret_args) { _cleanup_fdset_free_ FDSet *passed = NULL; bool do_exec = false; - int c, r; + int r; assert(argc >= 0); assert(argv); - while ((c = getopt_long(argc, argv, "hq", options, NULL)) >= 0) { + OptionParser state = {}; + const char *arg; + FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) switch (c) { - case 'h': + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case ARG_READY: + OPTION_LONG("ready", NULL, + "Inform the service manager about service start-up/reload completion"): arg_ready = true; break; - case ARG_RELOADING: + OPTION_LONG("reloading", NULL, + "Inform the service manager about configuration reloading"): arg_reloading = true; break; - case ARG_STOPPING: + OPTION_LONG("stopping", NULL, + "Inform the service manager about service shutdown"): arg_stopping = true; break; - case ARG_PID: + OPTION_FULL(OPTION_OPTIONAL_ARG, /* sc= */ 0, "pid", "PID", + "Set main PID of daemon"): pidref_done(&arg_pid); - if (isempty(optarg) || streq(optarg, "auto")) + if (isempty(arg) || streq(arg, "auto")) r = pidref_parent_if_applicable(&arg_pid); - else if (streq(optarg, "parent")) + else if (streq(arg, "parent")) r = pidref_set_parent(&arg_pid); - else if (streq(optarg, "self")) + else if (streq(arg, "self")) r = pidref_set_self(&arg_pid); else - r = pidref_set_pidstr(&arg_pid, optarg); + r = pidref_set_pidstr(&arg_pid, arg); if (r < 0) - return log_error_errno(r, "Failed to refer to --pid='%s': %m", optarg); - + return log_error_errno(r, "Failed to refer to --pid='%s': %m", arg); break; - case ARG_STATUS: - arg_status = optarg; + OPTION_LONG("uid", "USER", "Set user to send from"): + r = get_user_creds(&arg, &arg_uid, &arg_gid, NULL, NULL, 0); + if (r == -ESRCH) /* If the user doesn't exist, then accept it anyway as numeric */ + r = parse_uid(arg, &arg_uid); + if (r < 0) + return log_error_errno(r, "Can't resolve user %s: %m", arg); break; - case ARG_BOOTED: + OPTION_LONG("status", "TEXT", "Set status text"): + arg_status = arg; + break; + + OPTION_LONG("booted", NULL, "Check if the system was booted up with systemd"): arg_action = ACTION_BOOTED; break; - case ARG_UID: { - const char *u = optarg; - - r = get_user_creds(&u, &arg_uid, &arg_gid, NULL, NULL, 0); - if (r == -ESRCH) /* If the user doesn't exist, then accept it anyway as numeric */ - r = parse_uid(u, &arg_uid); - if (r < 0) - return log_error_errno(r, "Can't resolve user %s: %m", optarg); - - break; - } - - case ARG_NO_BLOCK: + OPTION_LONG("no-block", NULL, "Do not wait until operation finished"): arg_no_block = true; break; - case ARG_EXEC: + OPTION_LONG("exec", NULL, "Execute command line separated by ';' once done"): do_exec = true; break; - case ARG_FD: { + OPTION_LONG("fd", "FD", "Pass specified file descriptor along with the message"): { _cleanup_close_ int owned_fd = -EBADF; - int fdnr; - fdnr = parse_fd(optarg); + int fdnr = parse_fd(arg); if (fdnr < 0) - return log_error_errno(fdnr, "Failed to parse file descriptor: %s", optarg); + return log_error_errno(fdnr, "Failed to parse file descriptor: %s", arg); if (!passed) { /* Take possession of all passed fds */ @@ -310,33 +263,28 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_FDNAME: - if (!fdname_is_valid(optarg)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "File descriptor name invalid: %s", optarg); + OPTION_LONG("fdname", "NAME", "Name to assign to passed file descriptors"): + if (!fdname_is_valid(arg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "File descriptor name invalid: %s", arg); - if (free_and_strdup(&arg_fdname, optarg) < 0) + if (free_and_strdup(&arg_fdname, arg) < 0) return log_oom(); break; - case ARG_FORK: + OPTION_LONG("fork", NULL, "Receive notifications from child rather than sending them"): arg_action = ACTION_FORK; break; - case 'q': + OPTION('q', "quiet", NULL, "Do not show PID of child when forking"): arg_quiet = true; break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } - } bool have_env = arg_ready || arg_stopping || arg_reloading || arg_status || pidref_is_set(&arg_pid) || !fdset_isempty(arg_fds); + char **args = option_parser_get_args(&state, argc, argv); + switch (arg_action) { case ACTION_NOTIFY: { @@ -348,22 +296,22 @@ static int parse_argv(int argc, char *argv[]) { if (do_exec) { int i; - for (i = optind; i < argc; i++) - if (streq(argv[i], ";")) + for (i = 0; args[i]; i++) + if (streq(args[i], ";")) break; - if (i >= argc) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --exec is used argument list must contain ';' separator, refusing."); - if (i+1 == argc) + if (!args[i]) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --exec is used, argument list must contain ';' separator, refusing."); + if (!args[i + 1]) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Empty command line specified after ';' separator, refusing."); - arg_exec = strv_copy_n(argv + i + 1, argc - i - 1); + arg_exec = strv_copy(args + i + 1); if (!arg_exec) return log_oom(); - n_arg_env = i - optind; + n_arg_env = i; } else - n_arg_env = argc - optind; + n_arg_env = strv_length(args); have_env = have_env || n_arg_env > 0; if (!have_env) { @@ -376,7 +324,7 @@ static int parse_argv(int argc, char *argv[]) { } if (n_arg_env > 0) { - arg_env = strv_copy_n(argv + optind, n_arg_env); + arg_env = strv_copy_n(args, n_arg_env); if (!arg_env) return log_oom(); } @@ -388,13 +336,13 @@ static int parse_argv(int argc, char *argv[]) { } case ACTION_BOOTED: - if (argc > optind) + if (!strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--booted takes no parameters, refusing."); break; case ACTION_FORK: - if (optind >= argc) + if (strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--fork requires a command to be specified, refusing."); break; @@ -404,7 +352,10 @@ static int parse_argv(int argc, char *argv[]) { } if (have_env && arg_action != ACTION_NOTIFY) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "--ready, --reloading, --stopping, --pid=, --status=, --fd= may not be combined with --fork or --booted, refusing."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "--ready, --reloading, --stopping, --pid=, --status=, --fd= may not be combined with --fork or --booted, refusing."); + + *ret_args = args; return 1; } @@ -577,16 +528,17 @@ static int run(int argc, char* argv[]) { _cleanup_strv_free_ char **final_env = NULL; const char *our_env[10]; size_t i = 0; + char **args; int r; log_setup(); - r = parse_argv(argc, argv); + r = parse_argv(argc, argv, &args); if (r <= 0) return r; if (arg_action == ACTION_FORK) - return action_fork(argv + optind); + return action_fork(args); if (arg_action == ACTION_BOOTED) { r = sd_booted(); From 95ee4fd2cc4c3148033c802df9d4ee7d6c444308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 27 Feb 2026 20:22:04 +0100 Subject: [PATCH 11/15] notify: add one more assert We tend to get those preallocated array sizes wrong. In this case, the count was correct, but add the usual assert. --- src/notify/notify.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/notify/notify.c b/src/notify/notify.c index cdca928f90..d17df1eafa 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -604,6 +604,7 @@ static int run(int argc, char* argv[]) { } our_env[i++] = NULL; + assert(i <= ELEMENTSOF(our_env)); final_env = strv_env_merge((char**) our_env, arg_env); if (!final_env) From 57a1f1ab96a309f4e426db1ecfda4effb6078606 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 1 Mar 2026 11:16:02 +0100 Subject: [PATCH 12/15] bus-error: fix typo --- src/libsystemd/sd-bus/bus-error.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-error.h b/src/libsystemd/sd-bus/bus-error.h index 5aca67f006..32d29b3c8c 100644 --- a/src/libsystemd/sd-bus/bus-error.h +++ b/src/libsystemd/sd-bus/bus-error.h @@ -47,7 +47,7 @@ const char* _bus_error_message(const sd_bus_error *e, int error, char buf[static static const sd_bus_error_map * const CONCATENATE(errors ## _copy_, __COUNTER__) = errors; /* We use something exotic as end marker, to ensure people build the - * maps using the macsd-ros. */ + * maps using the macros. */ #define BUS_ERROR_MAP_END_MARKER -'x' BUS_ERROR_MAP_ELF_USE(bus_standard_errors); From bcaa5acf81b239f3912da91bff869cefb318820e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 2 Mar 2026 15:35:41 +0100 Subject: [PATCH 13/15] shared/format-table: add helpers to query and set column width --- src/shared/format-table.c | 69 +++++++++++++++++++++++++++++++++++++++ src/shared/format-table.h | 5 +++ 2 files changed, 74 insertions(+) diff --git a/src/shared/format-table.c b/src/shared/format-table.c index f400602b34..279e7fda68 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -2139,6 +2139,75 @@ static const char* table_data_rgap_underline(const TableData *d) { return NULL; } +int table_data_requested_width(Table *table, size_t column, size_t *ret) { + size_t width = 0; + int r; + + assert(table); + assert(ret); + + for (size_t row = 0; row < table_get_rows(table); row++) { + TableCell *cell = table_get_cell(table, row, column); + if (!cell) + continue; + + TableData *data = table_get_data(table, cell); + if (!data) + continue; + + size_t w; + + r = table_data_requested_width_height( + table, data, SIZE_MAX, &w, /* ret_height= */ NULL, /* have_soft= */ NULL); + if (r < 0) + return r; + + width = MAX(width, w); + } + + *ret = width; + return 0; +} + +int table_set_column_width(Table *t, size_t column, size_t width) { + int r = 0; + + assert(t); + + for (size_t row = 0; row < table_get_rows(t); row++) { + TableCell *cell = table_get_cell(t, row, column); + if (!cell) + continue; + + RET_GATHER(r, table_set_minimum_width(t, cell, width)); + } + + return r; +} + +int table_sync_column_width(Table *a, size_t column_a, Table *b, size_t column_b) { + size_t w1, w2; + int r; + + assert(a); + assert(b); + + /* Make both tables have specified columns of same width */ + + r = table_data_requested_width(a, column_a, &w1); + if (r < 0) + return log_error_errno(r, "Failed to query table column width: %m"); + + r = table_data_requested_width(b, column_b, &w2); + if (r < 0) + return log_error_errno(r, "Failed to query table column width: %m"); + + r = 0; + RET_GATHER(r, table_set_column_width(a, column_a, MAX(w1, w2))); + RET_GATHER(r, table_set_column_width(b, column_b, MAX(w1, w2))); + return r; +} + int table_print(Table *t, FILE *f) { size_t n_rows, *minimum_width, *maximum_width, display_columns, *requested_width, table_minimum_width, table_maximum_width, table_requested_width, table_effective_width, diff --git a/src/shared/format-table.h b/src/shared/format-table.h index 997ac20eb6..9a11fb7c30 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -141,6 +141,11 @@ int table_set_reverse(Table *t, size_t column, bool b); int table_hide_column_from_display_internal(Table *t, ...); #define table_hide_column_from_display(t, ...) table_hide_column_from_display_internal(t, __VA_ARGS__, SIZE_MAX) +int table_data_requested_width(Table *table, size_t column, size_t *ret); + +int table_set_column_width(Table *t, size_t column, size_t width); +int table_sync_column_width(Table *a, size_t column_a, Table *b, size_t column_b); + int table_print(Table *t, FILE *f); int table_format(Table *t, char **ret); From 8b89c6a64b21a954719b5026b87fce0bd6cca306 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 2 Mar 2026 23:40:51 +0100 Subject: [PATCH 14/15] Apply the same column width for different option groups This feel a bit like a hack, but it works OK. The width of the first column of verbs or options in different sections is measured and applied to the other tables. This makes the second column aligned. --- src/dissect/dissect.c | 3 +++ src/id128/id128.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index 76c3f90c7e..d36f31e5c7 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -140,6 +140,9 @@ static int help(void) { if (r < 0) return r; + /* Make the 1st column same width in both tables */ + (void) table_sync_column_width(options, 0, commands, 0); + printf("%1$s [OPTIONS...] IMAGE\n" "%1$s [OPTIONS...] --mount IMAGE PATH\n" "%1$s [OPTIONS...] --umount PATH\n" diff --git a/src/id128/id128.c b/src/id128/id128.c index 23a028b28c..ebed02913f 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -207,6 +207,9 @@ static int help(void) { if (r < 0) return r; + /* Make the 1st column same width in both tables */ + (void) table_sync_column_width(options, 0, verbs, 0); + printf("%s [OPTIONS...] COMMAND\n\n" "%sGenerate and print 128-bit identifiers.%s\n" "\nCommands:\n", From 1ddc5ae3e49f6db7d4014600fb9713696d6e0f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 3 Mar 2026 00:18:09 +0100 Subject: [PATCH 15/15] notify: add one more compiler suppression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Old gcc (<14) says: 2026-03-02T23:11:28.5676353Z In file included from ../src/notify/notify.c:30: 2026-03-02T23:11:28.5694607Z In function ‘strv_isempty’, 2026-03-02T23:11:28.5695481Z inlined from ‘action_fork’ at ../src/notify/notify.c:440:9, 2026-03-02T23:11:28.5696266Z inlined from ‘run’ at ../src/notify/notify.c:541:24, 2026-03-02T23:11:28.5696929Z inlined from ‘main’ at ../src/notify/notify.c:682:1: 2026-03-02T23:11:28.5697877Z ../src/basic/strv.h:108:23: error: ‘args’ may be used uninitialized [-Werror=maybe-uninitialized] 2026-03-02T23:11:28.5698655Z 108 | return !l || !*l; 2026-03-02T23:11:28.5699052Z | ^~ 2026-03-02T23:11:28.5700020Z ../src/notify/notify.c: In function ‘main’: 2026-03-02T23:11:28.5700681Z ../src/notify/notify.c:531:16: note: ‘args’ was declared here 2026-03-02T23:11:28.5701217Z 531 | char **args; 2026-03-02T23:11:28.5701574Z | ^~~~ 2026-03-02T23:11:28.5701960Z cc1: all warnings being treated as errors --- src/notify/notify.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/notify/notify.c b/src/notify/notify.c index d17df1eafa..5535296760 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -528,7 +528,7 @@ static int run(int argc, char* argv[]) { _cleanup_strv_free_ char **final_env = NULL; const char *our_env[10]; size_t i = 0; - char **args; + char **args = NULL; /* unnecessary initialization to appease gcc */ int r; log_setup(); @@ -536,6 +536,7 @@ static int run(int argc, char* argv[]) { r = parse_argv(argc, argv, &args); if (r <= 0) return r; + assert(args); if (arg_action == ACTION_FORK) return action_fork(args);