From b10d3631f36e4c45f6815f6461e4ab6e44553e03 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Apr 2026 10:32:52 +0200 Subject: [PATCH 1/9] options: drop redundant enum value declaration --- src/shared/options.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/options.h b/src/shared/options.h index 3d08390b33..889867bf6a 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -140,7 +140,7 @@ extern const Option __stop_SYSTEMD_OPTIONS[]; typedef enum OptionParserMode { /* The default mode. This is the implicit default and doesn't have to be specified. */ - OPTION_PARSER_NORMAL = 0, + OPTION_PARSER_NORMAL, /* Same as "+…" for getopt_long — only parse options before the first positional argument. */ OPTION_PARSER_STOP_AT_FIRST_NONOPTION, From bd2d2fec80f059eff279c76275f564323259ca37 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Apr 2026 10:37:48 +0200 Subject: [PATCH 2/9] options: tighten validation of argc/argv If argc/argv do no match a lot of our assumptions are invalid, hence let's check this explicitly once, instead of ignoring the issue. --- 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 644c7ae27c..0c3188bf78 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -76,9 +76,11 @@ int option_parse( if (state->optind == 0) { assert(state->mode >= 0 && state->mode < _OPTION_PARSER_MODE_MAX); - if (state->argc < 1 || strv_isempty(state->argv)) + if (state->argc < 1) return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "argv cannot be empty"); + assert_se((size_t) state->argc == strv_length(state->argv)); /* Make sure argc/argv are consistent */ + state->optind = state->positional_offset = 1; } From ce6a5f416432a86c702912a0edf18c2ca34a1314 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Apr 2026 10:33:46 +0200 Subject: [PATCH 3/9] options: add a proper state machine field Let's track the state of the option parser in an explicit 'state' field. Benefits: 1. We can make sure that a parser that got into a failure state will be invalidated for good (i.e. further operations are guaranteed to fail too). 2. As a side effect this cleans up the option_parse() return parameter handling: we'll now always initialize ret_option/ret_arg when returning >= 0, as per coding style. --- src/shared/options.c | 122 +++++++++++++++++++++++++++++-------------- src/shared/options.h | 14 ++++- 2 files changed, 95 insertions(+), 41 deletions(-) diff --git a/src/shared/options.c b/src/shared/options.c index 0c3188bf78..faefbe659e 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -72,23 +72,46 @@ int option_parse( const Option options_end[], OptionParser *state) { + /* We define this one early, since we use goto below, and need to guarantee its initialization */ + _cleanup_free_ char *_optname = NULL; /* allocated option name */ + int r; + + assert(state); + /* Check and initialize */ - if (state->optind == 0) { + switch (state->state) { + + case OPTION_PARSER_INIT: assert(state->mode >= 0 && state->mode < _OPTION_PARSER_MODE_MAX); - if (state->argc < 1) - return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "argv cannot be empty"); + if (state->argc < 1) { + r = log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "argv cannot be empty"); + goto fail; + } assert_se((size_t) state->argc == strv_length(state->argv)); /* Make sure argc/argv are consistent */ - state->optind = state->positional_offset = 1; + state->state = OPTION_PARSER_RUNNING; + break; + + case OPTION_PARSER_RUNNING: + case OPTION_PARSER_STOPPING: + break; + + case OPTION_PARSER_DONE: + goto done; + + case OPTION_PARSER_FAILED: + return log_error_errno(SYNTHETIC_ERRNO(ESTALE), "Option parser failed before, refusing."); + + default: + assert_not_reached(); } /* Look for the next 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; bool handling_positional_arg = false; @@ -96,27 +119,27 @@ int option_parse( /* Handle non-option parameters */ for (;;) { if (state->optind == state->argc) - goto finished; + goto done; if (streq(state->argv[state->optind], "--")) { /* No more options. Move "--" before positional args so that * the list of positional args is clean. */ shift_arg(state->argv, state->positional_offset++, state->optind++); - state->parsing_stopped = true; + goto done; } - if (state->parsing_stopped) - goto finished; + /* If we are in OPTION_PARSER_STOPPING state we only wanted to read one more "--" if + * there is one, nothing else, hence it's time to say goodbye now. */ + if (state->state == OPTION_PARSER_STOPPING) + goto done; if (state->argv[state->optind][0] == '-' && state->argv[state->optind][1] != '\0') /* Looks like we found an option parameter */ break; - if (state->mode == OPTION_PARSER_STOP_AT_FIRST_NONOPTION) { - state->parsing_stopped = true; - goto finished; - } + if (state->mode == OPTION_PARSER_STOP_AT_FIRST_NONOPTION) + goto done; if (state->mode == OPTION_PARSER_RETURN_POSITIONAL_ARGS) { handling_positional_arg = true; @@ -147,8 +170,10 @@ int option_parse( char *eq = strchr(state->argv[state->optind], '='); if (eq) { optname = _optname = strndup(state->argv[state->optind], eq - state->argv[state->optind]); - if (!_optname) - return log_oom(); + if (!_optname) { + r = log_oom(); + goto fail; + } /* joined argument */ optval = eq + 1; @@ -161,12 +186,16 @@ int option_parse( 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); + if (n_partial_matches == 0) { + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: unrecognized option '%s'", + program_invocation_short_name, optname); + goto fail; + } + if (n_partial_matches > 1) { + r = partial_match_error(options, options_end, optname, n_partial_matches); + goto fail; + } /* just one partial — good */ option = last_partial; @@ -195,15 +224,19 @@ int option_parse( if (state->short_option_offset > 0) { char optchar = state->argv[state->optind][state->short_option_offset]; - if (asprintf(&_optname, "-%c", optchar) < 0) - return log_oom(); + if (asprintf(&_optname, "-%c", optchar) < 0) { + r = log_oom(); + goto fail; + } 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 >= options_end) { + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: unrecognized option '%s'", + program_invocation_short_name, optname); + goto fail; + } if (option_is_metadata(option) || optchar != option->short_code) continue; @@ -225,15 +258,19 @@ int option_parse( assert(option); - if (!handling_positional_arg && 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 (!handling_positional_arg && optval && !option_takes_arg(option)) { + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: option '%s' doesn't allow an argument", + program_invocation_short_name, optname); + goto fail; + } if (!handling_positional_arg && !optval && option_arg_required(option)) { - if (!state->argv[state->optind + 1]) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: option '%s' requires an argument", - program_invocation_short_name, optname); + if (!state->argv[state->optind + 1]) { + r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: option '%s' requires an argument", + program_invocation_short_name, optname); + goto fail; + } optval = state->argv[state->optind + 1]; separate_optval = true; } @@ -252,16 +289,23 @@ int option_parse( } if (FLAGS_SET(option->flags, OPTION_STOPS_PARSING)) - state->parsing_stopped = true; + state->state = OPTION_PARSER_STOPPING; state->opt = option; state->arg = optval; return option->id; - finished: + done: + state->state = OPTION_PARSER_DONE; state->opt = NULL; state->arg = NULL; return 0; + + fail: + /* Invalidate the object for good on the first error */ + assert(r < 0); + state->state = OPTION_PARSER_FAILED; + return r; } char* option_parser_next_arg(const OptionParser *state) { @@ -294,7 +338,7 @@ char** option_parser_get_args(const OptionParser *state) { * original argv array, so it must not be freed or modified. */ assert(state->optind > 0); - assert(state->optind == state->argc || state->parsing_stopped); + assert(state->state == OPTION_PARSER_DONE); assert(state->positional_offset <= state->argc); return state->argv + state->positional_offset; @@ -302,7 +346,7 @@ char** option_parser_get_args(const OptionParser *state) { size_t option_parser_get_n_args(const OptionParser *state) { assert(state->optind > 0); - assert(state->optind == state->argc || state->parsing_stopped); + assert(state->state == OPTION_PARSER_DONE); assert(state->positional_offset <= state->argc); return state->argc - state->positional_offset; diff --git a/src/shared/options.h b/src/shared/options.h index 889867bf6a..81b92d2da5 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -152,14 +152,24 @@ typedef enum OptionParserMode { _OPTION_PARSER_MODE_MAX, } OptionParserMode; +typedef enum OptionParserState { + OPTION_PARSER_INIT, + OPTION_PARSER_RUNNING, + OPTION_PARSER_STOPPING, /* We processed an option with OPTION_STOPS_PARSING, and will eat up one + * more "--", but nothing else. */ + OPTION_PARSER_DONE, /* Option parsing completed (could be because we reached the end, or because + * "--" was fully processed, or because we hit a terminating option). */ + OPTION_PARSER_FAILED, /* We encountered a parse error, and terminated option parsing. */ + _OPTION_PARSER_MAX, +} OptionParserState; + typedef struct OptionParser { /* Those three should stay first so that it's possible to initialize the struct as { argc, argv } * or { argc, argv, mode }. */ int argc; /* The original argc. */ char **argv; /* The argv array, possibly reordered. */ OptionParserMode mode; - - bool parsing_stopped; /* We processed "--" or an option that terminates option parsing. */ + OptionParserState state; 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. From 1600978f5fc7ad0402d69e7d65c890fc5b10f9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 27 Apr 2026 22:56:50 +0200 Subject: [PATCH 4/9] shared/options: introduce "namespaces" for options This allows multiple option parsers to be defined in a single compilation unit. We put the OPTION_NAMESPACE("name") to split up the options. The basic implementation is similar to groups, except that groups only matter for help display, while namespaces matter for both help display and actual option parsing. When parsing, we locate the appropriate range between the beginning of options and the next namespace marker or between two namespace markers and only look at that range. --- src/shared/options.c | 74 +++++++++-- src/shared/options.h | 37 ++++-- src/test/test-options.c | 286 +++++++++++++++++++++++++++++----------- 3 files changed, 300 insertions(+), 97 deletions(-) diff --git a/src/shared/options.c b/src/shared/options.c index faefbe659e..364b57792c 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -21,7 +21,8 @@ static bool option_arg_required(const Option *opt) { static bool option_is_metadata(const Option *opt) { /* A metadata entry that is not a real option, like the group marker */ - return ASSERT_PTR(opt)->flags & (OPTION_GROUP_MARKER | + return ASSERT_PTR(opt)->flags & (OPTION_NAMESPACE_MARKER | + OPTION_GROUP_MARKER | OPTION_POSITIONAL_ENTRY | OPTION_HELP_ENTRY | OPTION_HELP_ENTRY_VERBATIM); @@ -81,7 +82,7 @@ int option_parse( /* Check and initialize */ switch (state->state) { - case OPTION_PARSER_INIT: + case OPTION_PARSER_INIT: { assert(state->mode >= 0 && state->mode < _OPTION_PARSER_MODE_MAX); if (state->argc < 1) { @@ -90,9 +91,34 @@ int option_parse( } assert_se((size_t) state->argc == strv_length(state->argv)); /* Make sure argc/argv are consistent */ + + /* Figure out the right range of options */ + bool in_ns = state->namespace == NULL; /* Are we currently in the section of the array that + * forms namespace ? The first part is the + * default unnamed namespace, so if the namespace was + * not specified, we are in it. */ + if (in_ns) + state->namespace_start = options; + + const Option *opt; + for (opt = options; opt < options_end; opt++) { + bool ns_marker = FLAGS_SET(opt->flags, OPTION_NAMESPACE_MARKER); + if (!in_ns) { + in_ns = ns_marker && streq(state->namespace, opt->long_code); + if (in_ns) + state->namespace_start = opt + 1; + continue; + } + if (ns_marker) + break; /* End of namespace */ + } + assert(state->namespace_start); + state->namespace_end = opt; + state->optind = state->positional_offset = 1; state->state = OPTION_PARSER_RUNNING; break; + } case OPTION_PARSER_RUNNING: case OPTION_PARSER_STOPPING: @@ -156,10 +182,10 @@ int option_parse( if (handling_positional_arg) /* We are supposed to return the positional arg to be handled. */ - for (option = options;; option++) { + for (option = state->namespace_start;; option++) { /* If OPTION_PARSER_RETURN_POSITIONAL_ARGS is specified, * OPTION_POSITIONAL must be used. */ - assert(option < options_end); + assert(option < state->namespace_end); if (FLAGS_SET(option->flags, OPTION_POSITIONAL_ENTRY)) break; @@ -184,8 +210,8 @@ int option_parse( 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) { + for (option = state->namespace_start;; option++) { + if (option >= state->namespace_end) { if (n_partial_matches == 0) { r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s: unrecognized option '%s'", @@ -193,7 +219,11 @@ int option_parse( goto fail; } if (n_partial_matches > 1) { - r = partial_match_error(options, options_end, optname, n_partial_matches); + r = partial_match_error( + state->namespace_start, + state->namespace_end, + optname, + n_partial_matches); goto fail; } @@ -230,8 +260,8 @@ int option_parse( } optname = _optname; - for (option = options;; option++) { - if (option >= options_end) { + for (option = state->namespace_start;; option++) { + if (option >= state->namespace_end) { r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s: unrecognized option '%s'", program_invocation_short_name, optname); @@ -354,7 +384,8 @@ size_t option_parser_get_n_args(const OptionParser *state) { char* option_get_synopsis(const char *prefix, const Option *opt, const char *joiner, bool show_metavar) { assert(opt); - assert(!FLAGS_SET(opt->flags, OPTION_GROUP_MARKER)); /* A group marker should not be displayed */ + assert(!(opt->flags & (OPTION_NAMESPACE_MARKER | + OPTION_GROUP_MARKER))); /* The markers should not be displayed */ if (!prefix) prefix = ""; @@ -405,9 +436,10 @@ char* option_get_synopsis(const char *prefix, const Option *opt, const char *joi option_arg_optional(opt) ? "]" : ""); } -int _option_parser_get_help_table( +int _option_parser_get_help_table_full( const Option options[], const Option options_end[], + const char *namespace, const char *group, Table **ret) { int r; @@ -418,11 +450,23 @@ int _option_parser_get_help_table( 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 - * if the group was not specified, we are in. */ + bool in_ns = namespace == NULL; /* Are we currently in the section of the array that forms namespace + * ? The first part is the default unnamed namespace, so + * if the namespace was not specified, we are in it. */ + + bool in_group = group == NULL; /* Are we currently in the section of the array that forms group + * ? The first part is the default group, so if the group was + * not specified, we are in it. */ for (const Option *opt = options; opt < options_end; opt++) { + bool ns_marker = FLAGS_SET(opt->flags, OPTION_NAMESPACE_MARKER); + if (!in_ns) { + in_ns = ns_marker && streq(namespace, opt->long_code); + continue; + } + if (ns_marker) + break; /* End of namespace */ + bool group_marker = FLAGS_SET(opt->flags, OPTION_GROUP_MARKER); if (!in_group) { in_group = group_marker && streq(group, opt->long_code); @@ -455,6 +499,8 @@ int _option_parser_get_help_table( return table_log_add_error(r); } + assert(!table_isempty(table)); /* The namespace or group were not found. Something is off. */ + table_set_header(table, false); *ret = TAKE_PTR(table); return 0; diff --git a/src/shared/options.h b/src/shared/options.h index 81b92d2da5..7d8507d0e5 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -4,13 +4,21 @@ #include "memory-util.h" #include "shared-forward.h" +/* Option namespace/group explanation: + * the list of options is split into namespaces, and a namespace is split into groups. + * By default, options defined in a single program are all placed in a single (unnamed) namespace + * and in a single (unnamed) group. OPTION_NAMESPACE() marks the beginning of a named namespace. + * OPTION_GROUP() marks the beginning of a named group. + */ + typedef enum OptionFlags { OPTION_OPTIONAL_ARG = 1U << 0, /* Same as optional_argument in getopt */ OPTION_POSITIONAL_ENTRY = 1U << 1, /* The "option" to handle positional arguments */ OPTION_STOPS_PARSING = 1U << 2, /* This option acts like "--" */ - OPTION_GROUP_MARKER = 1U << 3, /* Fake option entry to separate groups */ - OPTION_HELP_ENTRY = 1U << 4, /* Fake option entry to insert an additional help line */ - OPTION_HELP_ENTRY_VERBATIM = 1U << 5, /* Same, but use the long_code in the first column as written */ + OPTION_NAMESPACE_MARKER = 1U << 3, /* Fake option entry to separate namespaces */ + OPTION_GROUP_MARKER = 1U << 4, /* Fake option entry to separate groups */ + OPTION_HELP_ENTRY = 1U << 5, /* Fake option entry to insert an additional help line */ + OPTION_HELP_ENTRY_VERBATIM = 1U << 6, /* Same, but use the long_code in the first column as written */ } OptionFlags; typedef struct Option { @@ -41,9 +49,13 @@ typedef struct Option { }; \ case (0x100 + counter) +/* Magic entry in the table (which will not be returned) that designates the start of the namespace . + * The define is structured as 'case' so that it can be followed by ':' and indented appropriately. */ +#define OPTION_NAMESPACE(ns) \ + _OPTION(__COUNTER__, OPTION_NAMESPACE_MARKER, /* sc= */ 0, /* lc= */ ns, /* mv= */ NULL, /* d= */ 0u, /* h= */ NULL) + /* 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. - */ + * 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, /* d= */ 0u, /* h= */ NULL) @@ -164,11 +176,15 @@ typedef enum OptionParserState { } OptionParserState; typedef struct OptionParser { - /* Those three should stay first so that it's possible to initialize the struct as { argc, argv } - * or { argc, argv, mode }. */ + /* Those four should stay first so that it's possible to initialize the struct as { argc, argv } + * or { argc, argv, mode } or { argc, argv, mode, namespace }. */ int argc; /* The original argc. */ char **argv; /* The argv array, possibly reordered. */ OptionParserMode mode; + const char *namespace; /* The namespace, may be NULL. */ + + const Option *namespace_start, *namespace_end; /* The range of options that are part of our namespace. */ + OptionParserState state; int optind; /* Position of the parameter being handled. * 0 → option parsing hasn't been started yet. */ @@ -203,12 +219,15 @@ char** option_parser_get_args(const OptionParser *state); size_t option_parser_get_n_args(const OptionParser *state); char* option_get_synopsis(const char *prefix, const Option *opt, const char *joiner, bool show_metavar); -int _option_parser_get_help_table( +int _option_parser_get_help_table_full( const Option options[], const Option options_end[], + const char *namespace, const char *group, Table **ret); +#define option_parser_get_help_table_full(namespace, group, ret) \ + _option_parser_get_help_table_full(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, namespace, group, 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) + option_parser_get_help_table_full(/* namespace= */ NULL, group, ret) #define option_parser_get_help_table(ret) \ option_parser_get_help_table_group(/* group= */ NULL, ret) diff --git a/src/test/test-options.c b/src/test/test-options.c index 6a0744e673..a77964df4e 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -15,7 +15,8 @@ static void test_option_parse_one( const Option options[], const Entry *entries, char **remaining, - OptionParserMode mode) { + OptionParserMode mode, + const char *namespace) { _cleanup_free_ char *joined = strv_join(argv, ", "); log_debug("/* %s(%s) */", __func__, joined); @@ -32,7 +33,7 @@ static void test_option_parse_one( for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) n_entries++; - OptionParser opts = { argc, argv, mode }; + OptionParser opts = { argc, argv, mode, namespace }; for (int c; (c = option_parse(options, options + n_options, &opts)) != 0; ) { ASSERT_OK(c); ASSERT_NOT_NULL(opts.opt); @@ -90,6 +91,12 @@ TEST(option_parse) { { 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 }, + { 7, .long_code = "NS2", .flags = OPTION_NAMESPACE_MARKER }, + { 8, .short_code = 'h', .long_code = "help2" }, + { 9, .long_code = "version2" }, + { 10, .long_code = "NS3", .flags = OPTION_NAMESPACE_MARKER }, + { 11, .short_code = 'h', .long_code = "help3" }, + { 12, .long_code = "version3" }, {} }; @@ -97,7 +104,8 @@ TEST(option_parse) { options, NULL, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -110,7 +118,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--", @@ -124,7 +133,8 @@ TEST(option_parse) { "--help", "-h", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -138,7 +148,8 @@ TEST(option_parse) { "string2", "--", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -152,7 +163,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--help"), @@ -162,7 +174,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--help", @@ -175,7 +188,8 @@ TEST(option_parse) { }, STRV_MAKE("string1", "--help"), - OPTION_PARSER_STOP_AT_FIRST_NONOPTION); + OPTION_PARSER_STOP_AT_FIRST_NONOPTION, + NULL); test_option_parse_one(STRV_MAKE("arg0", "-h"), @@ -185,7 +199,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--help", @@ -202,7 +217,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "-h", @@ -219,7 +235,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -236,7 +253,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -253,7 +271,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -270,7 +289,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -287,7 +307,8 @@ TEST(option_parse) { "string2", "string3", "string4"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--required1", "reqarg1"), @@ -297,7 +318,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "-r", "reqarg1"), @@ -307,7 +329,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -320,7 +343,8 @@ TEST(option_parse) { }, STRV_MAKE("string1", "string2"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -331,7 +355,8 @@ TEST(option_parse) { STRV_MAKE("string1", "string2", "-r", "reqarg1"), - OPTION_PARSER_STOP_AT_FIRST_NONOPTION); + OPTION_PARSER_STOP_AT_FIRST_NONOPTION, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--optional1=optarg1"), @@ -341,7 +366,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_STOP_AT_FIRST_NONOPTION); + OPTION_PARSER_STOP_AT_FIRST_NONOPTION, + NULL); test_option_parse_one(STRV_MAKE("arg0", "--optional1", "string1"), @@ -351,7 +377,8 @@ TEST(option_parse) { {} }, STRV_MAKE("string1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "-ooptarg1"), @@ -361,7 +388,8 @@ TEST(option_parse) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "-o", "string1"), @@ -371,7 +399,8 @@ TEST(option_parse) { {} }, STRV_MAKE("string1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); test_option_parse_one(STRV_MAKE("arg0", "string1", @@ -441,7 +470,42 @@ TEST(option_parse) { "--help", "--required1", "--optional1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); + + /* Check that we can access options from NS2 */ + test_option_parse_one(STRV_MAKE("arg0", + "-h", /* This verifies that we're using the right namespace */ + "--help2", + "--version2", + "string1"), + options, + (Entry[]) { + { "help2" }, + { "help2" }, + { "version2" }, + {} + }, + STRV_MAKE("string1"), + OPTION_PARSER_NORMAL, + "NS2"); + + /* Check that we can access options from NS3 */ + test_option_parse_one(STRV_MAKE("arg0", + "-h", /* This verifies that we're using the right namespace */ + "--help3", + "--version3", + "string1"), + options, + (Entry[]) { + { "help3" }, + { "help3" }, + { "version3" }, + {} + }, + STRV_MAKE("string1"), + OPTION_PARSER_NORMAL, + "NS3"); } TEST(option_stops_parsing) { @@ -465,7 +529,8 @@ TEST(option_stops_parsing) { }, STRV_MAKE("--help", "foo"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Options before --exec are still parsed */ test_option_parse_one(STRV_MAKE("arg0", @@ -481,7 +546,8 @@ TEST(option_stops_parsing) { }, STRV_MAKE("--version", "bar"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* --exec with no trailing args */ test_option_parse_one(STRV_MAKE("arg0", @@ -492,7 +558,8 @@ TEST(option_stops_parsing) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* --exec after positional args */ test_option_parse_one(STRV_MAKE("arg0", @@ -509,7 +576,8 @@ TEST(option_stops_parsing) { "--help", "--required", "val"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* "--" 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 @@ -525,7 +593,8 @@ TEST(option_stops_parsing) { {} }, STRV_MAKE("--help"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* "--" before --exec: "--" terminates first, --exec is positional */ test_option_parse_one(STRV_MAKE("arg0", @@ -536,7 +605,8 @@ TEST(option_stops_parsing) { NULL, STRV_MAKE("--exec", "--help"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Multiple options then --exec then more option-like args */ test_option_parse_one(STRV_MAKE("arg0", @@ -555,7 +625,8 @@ TEST(option_stops_parsing) { STRV_MAKE("-h", "--required", "val2"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); } TEST(option_group_marker) { @@ -580,7 +651,8 @@ TEST(option_group_marker) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Check that group marker name is ignored */ test_option_parse_one(STRV_MAKE("arg0", @@ -593,7 +665,8 @@ TEST(option_group_marker) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Verify that the group marker is not mistaken for an option */ test_option_invalid_one(STRV_MAKE("arg0", @@ -621,7 +694,8 @@ TEST(option_group_marker) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Partial match with multiple candidates */ test_option_invalid_one(STRV_MAKE("arg0", @@ -645,7 +719,8 @@ TEST(option_optional_arg) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Long option without = does NOT consume the next arg */ test_option_parse_one(STRV_MAKE("arg0", @@ -656,7 +731,8 @@ TEST(option_optional_arg) { {} }, STRV_MAKE("foo.txt"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short option with inline arg */ test_option_parse_one(STRV_MAKE("arg0", @@ -667,7 +743,8 @@ TEST(option_optional_arg) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short option without inline arg does NOT consume the next arg */ test_option_parse_one(STRV_MAKE("arg0", @@ -678,7 +755,8 @@ TEST(option_optional_arg) { {} }, STRV_MAKE("foo.txt"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Optional arg option at end of argv */ test_option_parse_one(STRV_MAKE("arg0", @@ -689,7 +767,8 @@ TEST(option_optional_arg) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Mixed: optional arg with other options */ test_option_parse_one(STRV_MAKE("arg0", @@ -704,7 +783,8 @@ TEST(option_optional_arg) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short combo: -ho (h then o with no arg) */ test_option_parse_one(STRV_MAKE("arg0", @@ -716,7 +796,8 @@ TEST(option_optional_arg) { {} }, STRV_MAKE("pos1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short combo: -hobar (h then o with inline arg "bar") */ test_option_parse_one(STRV_MAKE("arg0", @@ -728,7 +809,8 @@ TEST(option_optional_arg) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); } /* Test the OPTION, OPTION_LONG, OPTION_SHORT, OPTION_FULL, OPTION_GROUP macros @@ -738,7 +820,8 @@ static void test_macros_parse_one( char **argv, const Entry *entries, char **remaining, - OptionParserMode mode) { + OptionParserMode mode, + const char *namespace) { _cleanup_free_ char *joined = strv_join(argv, ", "); log_debug("/* %s(%s) */", __func__, joined); @@ -752,7 +835,7 @@ static void test_macros_parse_one( for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) n_entries++; - OptionParser opts = { argc, argv, mode }; + OptionParser opts = { argc, argv, mode, namespace }; FOREACH_OPTION(c, &opts, assert_not_reached()) { log_debug("%c %s: %s=%s", @@ -814,9 +897,14 @@ static void test_macros_parse_one( OPTION_POSITIONAL: break; + OPTION_NAMESPACE("namespaced options"): {} + + OPTION('r', "required2", "ARG", "Required arg option"): + break; + default: log_error("Unexpected option id: %d", c); - ASSERT_TRUE(false); + assert_not_reached(); } } @@ -838,7 +926,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION: short form */ test_macros_parse_one(STRV_MAKE("arg0", @@ -848,7 +937,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_LONG: only accessible via long form */ test_macros_parse_one(STRV_MAKE("arg0", @@ -858,7 +948,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_SHORT: only accessible via short form */ test_macros_parse_one(STRV_MAKE("arg0", @@ -868,7 +959,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION with required arg: long --required=ARG */ test_macros_parse_one(STRV_MAKE("arg0", @@ -878,7 +970,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION with required arg: long --required ARG */ test_macros_parse_one(STRV_MAKE("arg0", @@ -888,7 +981,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION with required arg: short -r ARG */ test_macros_parse_one(STRV_MAKE("arg0", @@ -898,7 +992,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION with required arg: short -rARG */ test_macros_parse_one(STRV_MAKE("arg0", @@ -908,7 +1003,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_FULL with OPTION_OPTIONAL_ARG: long with = */ test_macros_parse_one(STRV_MAKE("arg0", @@ -918,7 +1014,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_FULL with OPTION_OPTIONAL_ARG: long without = doesn't consume next */ test_macros_parse_one(STRV_MAKE("arg0", @@ -928,7 +1025,8 @@ TEST(option_macros) { {} }, STRV_MAKE("pos1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_FULL with OPTION_OPTIONAL_ARG: short inline */ test_macros_parse_one(STRV_MAKE("arg0", @@ -938,7 +1036,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_FULL with OPTION_OPTIONAL_ARG: short without inline */ test_macros_parse_one(STRV_MAKE("arg0", @@ -948,7 +1047,8 @@ TEST(option_macros) { {} }, STRV_MAKE("pos1"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_FULL with OPTION_STOPS_PARSING: stops further option parsing */ test_macros_parse_one(STRV_MAKE("arg0", @@ -961,7 +1061,8 @@ TEST(option_macros) { }, STRV_MAKE("--help", "--version"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING: options before are still parsed */ test_macros_parse_one(STRV_MAKE("arg0", @@ -976,7 +1077,8 @@ TEST(option_macros) { }, STRV_MAKE("-h", "--debug"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING with "--": "--" after exec is still consumed */ test_macros_parse_one(STRV_MAKE("arg0", @@ -988,7 +1090,8 @@ TEST(option_macros) { {} }, STRV_MAKE("--help"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING with "--": "--" before exec takes precedence */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1000,7 +1103,8 @@ TEST(option_macros) { }, STRV_MAKE("--exec", "--help"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_GROUP: group marker is transparent to parsing, --debug in Advanced group works */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1010,7 +1114,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Mixed: all macro types together */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1039,7 +1144,8 @@ TEST(option_macros) { }, STRV_MAKE("pos1", "pos2"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short option combos with macros: -hv (help + verbose) */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1050,7 +1156,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short option combo with required arg: -hrval (help + required with arg "val") */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1061,7 +1168,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Short option combo with optional arg: -hoval (help + optional with arg "val") */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1072,7 +1180,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING then "--": "--" is still consumed after exec */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1088,7 +1197,8 @@ TEST(option_macros) { }, STRV_MAKE("--version", "-h"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING then later "--": "--" is not consumed */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1105,7 +1215,8 @@ TEST(option_macros) { STRV_MAKE("--version", "--", "-h"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* OPTION_STOPS_PARSING then "--" twice: second "--" is not consumed */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1123,7 +1234,8 @@ TEST(option_macros) { STRV_MAKE("--", "--version", "-h"), - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* Basic OPTION_POSITIONAL use */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1139,7 +1251,8 @@ TEST(option_macros) { {} }, NULL, - OPTION_PARSER_RETURN_POSITIONAL_ARGS); + OPTION_PARSER_RETURN_POSITIONAL_ARGS, + NULL); /* OPTION_POSITIONAL combined with OPTION_STOPS_PARSING */ test_macros_parse_one(STRV_MAKE("arg0", @@ -1154,7 +1267,30 @@ TEST(option_macros) { {} }, STRV_MAKE("arg2"), - OPTION_PARSER_RETURN_POSITIONAL_ARGS); + OPTION_PARSER_RETURN_POSITIONAL_ARGS, + NULL); + + /* Second namespace, OPTION: long form */ + test_macros_parse_one(STRV_MAKE("arg0", + "--required2=arg"), + (Entry[]) { + { "required2", "arg" }, + {} + }, + NULL, + OPTION_PARSER_NORMAL, + "namespaced options"); + + /* Second namespace, OPTION: short form */ + test_macros_parse_one(STRV_MAKE("arg0", + "-rarg"), + (Entry[]) { + { "required2", "arg" }, + {} + }, + NULL, + OPTION_PARSER_NORMAL, + "namespaced options"); } /* Test the pattern used by nspawn's --user: an optional-arg option that also @@ -1176,7 +1312,8 @@ TEST(option_optional_arg_consume) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* --user without arg: next arg is an option, so no consumption */ test_option_parse_one(STRV_MAKE("arg0", @@ -1189,7 +1326,8 @@ TEST(option_optional_arg_consume) { {} }, NULL, - OPTION_PARSER_NORMAL); + OPTION_PARSER_NORMAL, + NULL); /* --user without arg: next arg is positional (doesn't start with -). * The option parser returns NULL for the arg. The caller would then From ff902c7ccda2165f7c33231a0174384f0d1331b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 27 Apr 2026 23:58:24 +0200 Subject: [PATCH 5/9] run: reorder switch cases to match help() output order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both parse_argv() and parse_argv_sudo_mode() handled options in an order that no longer matched the help text. Reorder the case statements so the source order mirrors what the user sees in --help. In parse_argv_sudo_mode(), drop the case 'i' → ARG_VIA_SHELL fall-through so the cases can be sequenced independently; 'i' now sets arg_via_shell directly. Co-developed-by: Claude Opus 4.7 --- src/run/run.c | 252 +++++++++++++++++++++++++------------------------- 1 file changed, 127 insertions(+), 125 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 596e128267..60eaeb12d5 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -433,6 +433,17 @@ static int parse_argv(int argc, char *argv[]) { arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; break; + case 'H': + arg_transport = BUS_TRANSPORT_REMOTE; + arg_host = optarg; + break; + + case 'M': + r = parse_machine_argument(optarg, &arg_host, &arg_transport); + if (r < 0) + return r; + break; + case 'C': r = capsule_name_is_valid(optarg); if (r < 0) @@ -453,6 +464,12 @@ static int parse_argv(int argc, char *argv[]) { arg_unit = optarg; break; + case 'p': + if (strv_extend(&arg_property, optarg) < 0) + return log_oom(); + + break; + case ARG_DESCRIPTION: r = free_and_strdup_warn(&arg_description, optarg); if (r < 0) @@ -475,23 +492,20 @@ static int parse_argv(int argc, char *argv[]) { return r; break; - case ARG_SEND_SIGHUP: - arg_send_sighup = true; + case ARG_NO_BLOCK: + arg_no_block = true; break; case 'r': arg_remain_after_exit = true; break; - case 'H': - arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; + case ARG_WAIT: + arg_wait = true; break; - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); - if (r < 0) - return r; + case ARG_SEND_SIGHUP: + arg_send_sighup = true; break; case ARG_SERVICE_TYPE: @@ -516,6 +530,44 @@ static int parse_argv(int argc, char *argv[]) { arg_nice_set = true; break; + case ARG_WORKING_DIRECTORY: + r = parse_path_argument(optarg, true, &arg_working_directory); + if (r < 0) + return r; + + same_dir = false; + break; + + case 'd': { + _cleanup_free_ char *p = NULL; + + r = safe_getcwd(&p); + if (r < 0) + return log_error_errno(r, "Failed to get current working directory: %m"); + + if (empty_or_root(p)) + arg_working_directory = mfree(arg_working_directory); + else + free_and_replace(arg_working_directory, p); + + same_dir = true; + break; + } + + case ARG_ROOT_DIRECTORY: + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root_directory); + if (r < 0) + return r; + + break; + + case 'R': + r = free_and_strdup_warn(&arg_root_directory, "/"); + if (r < 0) + return r; + + break; + case 'E': r = strv_env_replace_strdup_passthrough(&arg_environment, optarg); if (r < 0) @@ -523,14 +575,8 @@ static int parse_argv(int argc, char *argv[]) { break; - case 'p': - if (strv_extend(&arg_property, optarg) < 0) - return log_oom(); - - break; - + case 't': /* --pty (and --tty deprecated alias) */ case 'T': /* --pty-late */ - case 't': /* --pty */ arg_stdio |= ARG_STDIO_PTY; arg_pty_late = c == 'T'; break; @@ -556,6 +602,59 @@ static int parse_argv(int argc, char *argv[]) { return log_error_errno(arg_output, "Unknown output format '%s'.", optarg); break; + case ARG_JSON: + r = parse_json_argument(optarg, &arg_json_format_flags); + if (r <= 0) + return r; + break; + + case 'G': + arg_aggressive_gc = true; + break; + + case 'S': + arg_shell = true; + break; + + case ARG_JOB_MODE: + if (streq(optarg, "help")) + return DUMP_STRING_TABLE(job_mode, JobMode, _JOB_MODE_MAX); + + r = job_mode_from_string(optarg); + if (r < 0) + return log_error_errno(r, "Invalid job mode: %s", optarg); + + arg_job_mode = r; + break; + + case ARG_IGNORE_FAILURE: + arg_ignore_failure = true; + break; + + case ARG_BACKGROUND: + r = parse_background_argument(optarg, &arg_background); + if (r < 0) + return r; + break; + + case ARG_NO_PAGER: + arg_pager_flags |= PAGER_DISABLE; + break; + + case ARG_PATH_PROPERTY: + + if (strv_extend(&arg_path_property, optarg) < 0) + return log_oom(); + + break; + + case ARG_SOCKET_PROPERTY: + + if (strv_extend(&arg_socket_property, optarg) < 0) + return log_oom(); + + break; + case ARG_ON_ACTIVE: r = add_timer_property("OnActiveSec", optarg); if (r < 0) @@ -653,105 +752,6 @@ static int parse_argv(int argc, char *argv[]) { "OnCalendar="); break; - case ARG_PATH_PROPERTY: - - if (strv_extend(&arg_path_property, optarg) < 0) - return log_oom(); - - break; - - case ARG_SOCKET_PROPERTY: - - if (strv_extend(&arg_socket_property, optarg) < 0) - return log_oom(); - - break; - - case ARG_NO_BLOCK: - arg_no_block = true; - break; - - case ARG_WAIT: - arg_wait = true; - break; - - case ARG_WORKING_DIRECTORY: - r = parse_path_argument(optarg, true, &arg_working_directory); - if (r < 0) - return r; - - same_dir = false; - break; - - case 'd': { - _cleanup_free_ char *p = NULL; - - r = safe_getcwd(&p); - if (r < 0) - return log_error_errno(r, "Failed to get current working directory: %m"); - - if (empty_or_root(p)) - arg_working_directory = mfree(arg_working_directory); - else - free_and_replace(arg_working_directory, p); - - same_dir = true; - break; - } - - case ARG_ROOT_DIRECTORY: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root_directory); - if (r < 0) - return r; - - break; - - case 'R': - r = free_and_strdup_warn(&arg_root_directory, "/"); - if (r < 0) - return r; - - break; - - case 'G': - arg_aggressive_gc = true; - break; - - case 'S': - arg_shell = true; - break; - - case ARG_JOB_MODE: - if (streq(optarg, "help")) - return DUMP_STRING_TABLE(job_mode, JobMode, _JOB_MODE_MAX); - - r = job_mode_from_string(optarg); - if (r < 0) - return log_error_errno(r, "Invalid job mode: %s", optarg); - - arg_job_mode = r; - break; - - case ARG_IGNORE_FAILURE: - arg_ignore_failure = true; - break; - - case ARG_BACKGROUND: - r = parse_background_argument(optarg, &arg_background); - if (r < 0) - return r; - break; - - case ARG_NO_PAGER: - arg_pager_flags |= PAGER_DISABLE; - break; - - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); - if (r <= 0) - return r; - break; - case '?': return -EINVAL; @@ -1066,6 +1066,18 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { break; + case ARG_VIA_SHELL: + arg_via_shell = true; + break; + + case 'i': + r = free_and_strdup_warn(&arg_working_directory, "~"); + if (r < 0) + return r; + + arg_via_shell = true; + break; + case ARG_SETENV: r = strv_env_replace_strdup_passthrough(&arg_environment, optarg); if (r < 0) @@ -1113,16 +1125,6 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { break; - case 'i': - r = free_and_strdup_warn(&arg_working_directory, "~"); - if (r < 0) - return r; - - _fallthrough_; - case ARG_VIA_SHELL: - arg_via_shell = true; - break; - case ARG_EMPOWER: arg_empower = true; break; From 4c0e12b3b922f99a9be09e3be9df6a7a44b5db04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Apr 2026 00:04:50 +0200 Subject: [PATCH 6/9] run: convert parse_argv() to OPTION macros --system is now documented (fixup for 66b1e746055b9c56fd72c0451a4cfb2b06cf3f20). --capsule is now documented (fixup for 759b3c082d463a488235592df45cbebefbe1ad5c). --help output is generally the same, except for the formatting changes related to use of the new output helpers and common option macros. Co-developed-by: Claude Opus 4.7 --- src/run/run.c | 419 +++++++++++++++++--------------------------------- 1 file changed, 143 insertions(+), 276 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 60eaeb12d5..36ebe32f2d 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -40,9 +40,11 @@ #include "format-table.h" #include "format-util.h" #include "fs-util.h" +#include "help-util.h" #include "hostname-util.h" #include "log.h" #include "main-func.h" +#include "options.h" #include "osc-context.h" #include "pager.h" #include "parse-argument.h" @@ -139,80 +141,43 @@ STATIC_DESTRUCTOR_REGISTER(arg_shell_prompt_prefix, freep); STATIC_DESTRUCTOR_REGISTER(arg_area, freep); static int help(void) { - _cleanup_free_ char *link = NULL; int r; pager_open(arg_pager_flags); - r = terminal_urlify_man("systemd-run", "1", &link); - if (r < 0) - return log_oom(); + static const char* const groups[] = { + NULL, + "Path options", + "Socket options", + "Timer options", + }; - printf("%1$s [OPTIONS...] COMMAND [ARGUMENTS...]\n" - "\n%5$sRun the specified command in a transient scope or service.%6$s\n\n" - " -h --help Show this help\n" - " --version Show package version\n" - " --no-ask-password Do not prompt for password\n" - " --user Run as user unit\n" - " -H --host=[USER@]HOST Operate on remote host\n" - " -M --machine=CONTAINER Operate on local container\n" - " --scope Run this as scope rather than service\n" - " -u --unit=UNIT Run under the specified unit name\n" - " -p --property=NAME=VALUE Set service or scope unit property\n" - " --description=TEXT Description for unit\n" - " --slice=SLICE Run in the specified slice\n" - " --slice-inherit Inherit the slice from the caller\n" - " --expand-environment=BOOL Control expansion of environment variables\n" - " --no-block Do not wait until operation finished\n" - " -r --remain-after-exit Leave service around until explicitly stopped\n" - " --wait Wait until service stopped again\n" - " --send-sighup Send SIGHUP when terminating\n" - " --service-type=TYPE Service type\n" - " --uid=USER Run as system user\n" - " --gid=GROUP Run as system group\n" - " --nice=NICE Nice level\n" - " --working-directory=PATH Set working directory\n" - " -d --same-dir Inherit working directory from caller\n" - " --root-directory=PATH Set root directory\n" - " -R --same-root-dir Inherit root directory from caller\n" - " -E --setenv=NAME[=VALUE] Set environment variable\n" - " -t --pty Run service on pseudo TTY as STDIN/STDOUT/\n" - " STDERR\n" - " -T --pty-late Just like --pty, but leave TTY access to\n" - " agents until unit is started up\n" - " -P --pipe Pass STDIN/STDOUT/STDERR directly to service\n" - " -q --quiet Suppress information messages during runtime\n" - " -v --verbose Show unit logs while executing operation\n" - " --output=STRING Controls formatting of verbose logs, see\n" - " journalctl for valid values\n" - " --json=pretty|short|off Print unit name and invocation id as JSON\n" - " -G --collect Unload unit after it ran, even when failed\n" - " -S --shell Invoke a $SHELL interactively\n" - " --job-mode=MODE Specify how to deal with already queued jobs,\n" - " when queueing a new job\n" - " --ignore-failure Ignore the exit status of the invoked process\n" - " --background=COLOR Set ANSI color for background\n" - " --no-pager Do not pipe output into a pager\n" - "\n%3$sPath options:%4$s\n" - " --path-property=NAME=VALUE Set path unit property\n" - "\n%3$sSocket options:%4$s\n" - " --socket-property=NAME=VALUE Set socket unit property\n" - "\n%3$sTimer options:%4$s\n" - " --on-active=SECONDS Run after SECONDS delay\n" - " --on-boot=SECONDS Run SECONDS after machine was booted up\n" - " --on-startup=SECONDS Run SECONDS after systemd activation\n" - " --on-unit-active=SECONDS Run SECONDS after the last activation\n" - " --on-unit-inactive=SECONDS Run SECONDS after the last deactivation\n" - " --on-calendar=SPEC Realtime timer\n" - " --on-timezone-change Run when the timezone changes\n" - " --on-clock-change Run when the realtime clock jumps\n" - " --timer-property=NAME=VALUE Set timer unit property\n" - "\nSee the %2$s for details.\n", - program_invocation_short_name, - link, - ansi_underline(), ansi_normal(), - ansi_highlight(), ansi_normal()); + _cleanup_(table_unref_many) Table *tables[ELEMENTSOF(groups) + 1] = {}; + for (size_t i = 0; i < ELEMENTSOF(groups); i++) { + r = option_parser_get_help_table_group(groups[i], &tables[i]); + if (r < 0) + return r; + } + + (void) table_sync_column_widths(0, tables[0], tables[1], tables[2], tables[3]); + + help_cmdline("[OPTIONS...] COMMAND [ARGUMENTS...]"); + help_abstract("Run the specified command in a transient scope or service."); + + for (size_t i = 0; i < ELEMENTSOF(groups); i++) { + _cleanup_free_ char *title = strjoin(groups[i] ?: "Options", ":"); + if (!title) + return log_oom(); + + help_section(title); + + r = table_print_or_warn(tables[i]); + if (r < 0) + return r; + } + + help_man_page_reference("systemd-run", "1"); return 0; } @@ -306,239 +271,142 @@ static char** make_login_shell_cmdline(const char *shell) { } static int parse_argv(int argc, char *argv[]) { - - enum { - ARG_VERSION = 0x100, - ARG_USER, - ARG_SYSTEM, - ARG_SCOPE, - ARG_DESCRIPTION, - ARG_SLICE, - ARG_SLICE_INHERIT, - ARG_EXPAND_ENVIRONMENT, - ARG_SEND_SIGHUP, - ARG_SERVICE_TYPE, - ARG_EXEC_USER, - ARG_EXEC_GROUP, - ARG_NICE, - ARG_OUTPUT, - ARG_ON_ACTIVE, - ARG_ON_BOOT, - ARG_ON_STARTUP, - ARG_ON_UNIT_ACTIVE, - ARG_ON_UNIT_INACTIVE, - ARG_ON_CALENDAR, - ARG_ON_TIMEZONE_CHANGE, - ARG_ON_CLOCK_CHANGE, - ARG_TIMER_PROPERTY, - ARG_PATH_PROPERTY, - ARG_SOCKET_PROPERTY, - ARG_NO_BLOCK, - ARG_NO_ASK_PASSWORD, - ARG_WAIT, - ARG_WORKING_DIRECTORY, - ARG_ROOT_DIRECTORY, - ARG_JOB_MODE, - ARG_IGNORE_FAILURE, - ARG_BACKGROUND, - ARG_NO_PAGER, - ARG_JSON, - }; - - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, ARG_VERSION }, - { "user", no_argument, NULL, ARG_USER }, - { "system", no_argument, NULL, ARG_SYSTEM }, - { "capsule", required_argument, NULL, 'C' }, - { "scope", no_argument, NULL, ARG_SCOPE }, - { "unit", required_argument, NULL, 'u' }, - { "description", required_argument, NULL, ARG_DESCRIPTION }, - { "slice", required_argument, NULL, ARG_SLICE }, - { "slice-inherit", no_argument, NULL, ARG_SLICE_INHERIT }, - { "remain-after-exit", no_argument, NULL, 'r' }, - { "expand-environment", required_argument, NULL, ARG_EXPAND_ENVIRONMENT }, - { "send-sighup", no_argument, NULL, ARG_SEND_SIGHUP }, - { "host", required_argument, NULL, 'H' }, - { "machine", required_argument, NULL, 'M' }, - { "service-type", required_argument, NULL, ARG_SERVICE_TYPE }, - { "wait", no_argument, NULL, ARG_WAIT }, - { "uid", required_argument, NULL, ARG_EXEC_USER }, - { "gid", required_argument, NULL, ARG_EXEC_GROUP }, - { "nice", required_argument, NULL, ARG_NICE }, - { "setenv", required_argument, NULL, 'E' }, - { "property", required_argument, NULL, 'p' }, - { "tty", no_argument, NULL, 't' }, /* deprecated alias */ - { "pty", no_argument, NULL, 't' }, - { "pty-late", no_argument, NULL, 'T' }, - { "pipe", no_argument, NULL, 'P' }, - { "quiet", no_argument, NULL, 'q' }, - { "verbose", no_argument, NULL, 'v' }, - { "output", required_argument, NULL, ARG_OUTPUT }, - { "on-active", required_argument, NULL, ARG_ON_ACTIVE }, - { "on-boot", required_argument, NULL, ARG_ON_BOOT }, - { "on-startup", required_argument, NULL, ARG_ON_STARTUP }, - { "on-unit-active", required_argument, NULL, ARG_ON_UNIT_ACTIVE }, - { "on-unit-inactive", required_argument, NULL, ARG_ON_UNIT_INACTIVE }, - { "on-calendar", required_argument, NULL, ARG_ON_CALENDAR }, - { "on-timezone-change", no_argument, NULL, ARG_ON_TIMEZONE_CHANGE }, - { "on-clock-change", no_argument, NULL, ARG_ON_CLOCK_CHANGE }, - { "timer-property", required_argument, NULL, ARG_TIMER_PROPERTY }, - { "path-property", required_argument, NULL, ARG_PATH_PROPERTY }, - { "socket-property", required_argument, NULL, ARG_SOCKET_PROPERTY }, - { "no-block", no_argument, NULL, ARG_NO_BLOCK }, - { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, - { "collect", no_argument, NULL, 'G' }, - { "working-directory", required_argument, NULL, ARG_WORKING_DIRECTORY }, - { "same-dir", no_argument, NULL, 'd' }, - { "root-directory", required_argument, NULL, ARG_ROOT_DIRECTORY }, - { "same-root-dir", no_argument, NULL, 'R' }, - { "shell", no_argument, NULL, 'S' }, - { "job-mode", required_argument, NULL, ARG_JOB_MODE }, - { "ignore-failure", no_argument, NULL, ARG_IGNORE_FAILURE }, - { "background", required_argument, NULL, ARG_BACKGROUND }, - { "no-pager", no_argument, NULL, ARG_NO_PAGER }, - { "json", required_argument, NULL, ARG_JSON }, - {}, - }; - bool with_trigger = false, same_dir = false; - int r, c; + int r; assert(argc >= 0); assert(argv); - /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() - * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ - optind = 0; - while ((c = getopt_long(argc, argv, "+hrC:H:M:E:p:tTPqvGdSu:", options, NULL)) >= 0) + OptionParser opts = { argc, argv, OPTION_PARSER_STOP_AT_FIRST_NONOPTION }; + FOREACH_OPTION(c, &opts, /* on_error= */ return c) switch (c) { - case 'h': + OPTION_COMMON_HELP: return help(); - case ARG_VERSION: + OPTION_COMMON_VERSION: return version(); - case ARG_NO_ASK_PASSWORD: + OPTION_COMMON_NO_ASK_PASSWORD: arg_ask_password = false; break; - case ARG_USER: + OPTION_LONG("user", NULL, "Run as user unit"): arg_runtime_scope = RUNTIME_SCOPE_USER; break; - case ARG_SYSTEM: + OPTION_LONG("system", NULL, "Talk to the service manager (implied default)"): arg_runtime_scope = RUNTIME_SCOPE_SYSTEM; break; - case 'H': + OPTION_COMMON_HOST: arg_transport = BUS_TRANSPORT_REMOTE; - arg_host = optarg; + arg_host = opts.arg; break; - case 'M': - r = parse_machine_argument(optarg, &arg_host, &arg_transport); + OPTION_COMMON_MACHINE: + r = parse_machine_argument(opts.arg, &arg_host, &arg_transport); if (r < 0) return r; break; - case 'C': - r = capsule_name_is_valid(optarg); + OPTION('C', "capsule", "NAME", "Operate on specified capsule"): + r = capsule_name_is_valid(opts.arg); if (r < 0) - return log_error_errno(r, "Unable to validate capsule name '%s': %m", optarg); + return log_error_errno(r, "Unable to validate capsule name '%s': %m", opts.arg); if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", optarg); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capsule name: %s", opts.arg); - arg_host = optarg; + arg_host = opts.arg; arg_transport = BUS_TRANSPORT_CAPSULE; arg_runtime_scope = RUNTIME_SCOPE_USER; break; - case ARG_SCOPE: + OPTION_LONG("scope", NULL, "Run this as scope rather than service"): arg_scope = true; break; - case 'u': - arg_unit = optarg; + OPTION('u', "unit", "UNIT", "Run under the specified unit name"): + arg_unit = opts.arg; break; - case 'p': - if (strv_extend(&arg_property, optarg) < 0) + OPTION('p', "property", "NAME=VALUE", "Set service or scope unit property"): + if (strv_extend(&arg_property, opts.arg) < 0) return log_oom(); - break; - case ARG_DESCRIPTION: - r = free_and_strdup_warn(&arg_description, optarg); + OPTION_LONG("description", "TEXT", "Description for unit"): + r = free_and_strdup_warn(&arg_description, opts.arg); if (r < 0) return r; break; - case ARG_SLICE: - r = free_and_strdup_warn(&arg_slice, optarg); + OPTION_LONG("slice", "SLICE", "Run in the specified slice"): + r = free_and_strdup_warn(&arg_slice, opts.arg); if (r < 0) return r; break; - case ARG_SLICE_INHERIT: + OPTION_LONG("slice-inherit", NULL, "Inherit the slice from the caller"): arg_slice_inherit = true; break; - case ARG_EXPAND_ENVIRONMENT: - r = parse_boolean_argument("--expand-environment=", optarg, &arg_expand_environment); + OPTION_LONG("expand-environment", "BOOL", + "Control expansion of environment variables"): + r = parse_boolean_argument("--expand-environment=", opts.arg, &arg_expand_environment); if (r < 0) return r; break; - case ARG_NO_BLOCK: + OPTION_LONG("no-block", NULL, "Do not wait until operation finished"): arg_no_block = true; break; - case 'r': + OPTION('r', "remain-after-exit", NULL, + "Leave service around until explicitly stopped"): arg_remain_after_exit = true; break; - case ARG_WAIT: + OPTION_LONG("wait", NULL, "Wait until service stopped again"): arg_wait = true; break; - case ARG_SEND_SIGHUP: + OPTION_LONG("send-sighup", NULL, "Send SIGHUP when terminating"): arg_send_sighup = true; break; - case ARG_SERVICE_TYPE: - arg_service_type = optarg; + OPTION_LONG("service-type", "TYPE", "Service type"): + arg_service_type = opts.arg; break; - case ARG_EXEC_USER: - r = free_and_strdup_warn(&arg_exec_user, optarg); + OPTION_LONG("uid", "USER", "Run as system user"): + r = free_and_strdup_warn(&arg_exec_user, opts.arg); if (r < 0) return r; break; - case ARG_EXEC_GROUP: - arg_exec_group = optarg; + OPTION_LONG("gid", "GROUP", "Run as system group"): + arg_exec_group = opts.arg; break; - case ARG_NICE: - r = parse_nice(optarg, &arg_nice); + OPTION_LONG("nice", "NICE", "Nice level"): + r = parse_nice(opts.arg, &arg_nice); if (r < 0) - return log_error_errno(r, "Failed to parse nice value: %s", optarg); + return log_error_errno(r, "Failed to parse nice value: %s", opts.arg); arg_nice_set = true; break; - case ARG_WORKING_DIRECTORY: - r = parse_path_argument(optarg, true, &arg_working_directory); + OPTION_LONG("working-directory", "PATH", "Set working directory"): + r = parse_path_argument(opts.arg, true, &arg_working_directory); if (r < 0) return r; same_dir = false; break; - case 'd': { + OPTION('d', "same-dir", NULL, "Inherit working directory from caller"): { _cleanup_free_ char *p = NULL; r = safe_getcwd(&p); @@ -554,151 +422,156 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_ROOT_DIRECTORY: - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_root_directory); + OPTION_LONG("root-directory", "PATH", "Set root directory"): + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_root_directory); if (r < 0) return r; - break; - case 'R': + OPTION('R', "same-root-dir", NULL, "Inherit root directory from caller"): r = free_and_strdup_warn(&arg_root_directory, "/"); if (r < 0) return r; - break; - case 'E': - r = strv_env_replace_strdup_passthrough(&arg_environment, optarg); + OPTION('E', "setenv", "NAME[=VALUE]", "Set environment variable"): + r = strv_env_replace_strdup_passthrough(&arg_environment, opts.arg); if (r < 0) - return log_error_errno(r, "Cannot assign environment variable %s: %m", optarg); - + return log_error_errno(r, "Cannot assign environment variable %s: %m", opts.arg); break; - case 't': /* --pty (and --tty deprecated alias) */ - case 'T': /* --pty-late */ + OPTION_LONG("tty", NULL, NULL): {} /* deprecated alias for --pty */ + OPTION('t', "pty", NULL, + "Run service on pseudo TTY as STDIN/STDOUT/STDERR"): {} + OPTION('T', "pty-late", NULL, + "Just like --pty, but leave TTY access to agents until unit is started up"): arg_stdio |= ARG_STDIO_PTY; - arg_pty_late = c == 'T'; + arg_pty_late = opts.opt->short_code == 'T'; break; - case 'P': /* --pipe */ + OPTION('P', "pipe", NULL, "Pass STDIN/STDOUT/STDERR directly to service"): arg_stdio |= ARG_STDIO_DIRECT; break; - case 'q': + OPTION('q', "quiet", NULL, "Suppress information messages during runtime"): arg_quiet = true; break; - case 'v': + OPTION('v', "verbose", NULL, "Show unit logs while executing operation"): arg_verbose = true; break; - case ARG_OUTPUT: - if (streq(optarg, "help")) + OPTION_LONG("output", "MODE", + "Controls formatting of verbose logs, see journalctl for valid values"): + if (streq(opts.arg, "help")) return DUMP_STRING_TABLE(output_mode, OutputMode, _OUTPUT_MODE_MAX); - arg_output = output_mode_from_string(optarg); + arg_output = output_mode_from_string(opts.arg); if (arg_output < 0) - return log_error_errno(arg_output, "Unknown output format '%s'.", optarg); + return log_error_errno(arg_output, "Unknown output format '%s'.", opts.arg); break; - case ARG_JSON: - r = parse_json_argument(optarg, &arg_json_format_flags); + OPTION_COMMON_JSON: + r = parse_json_argument(opts.arg, &arg_json_format_flags); if (r <= 0) return r; break; - case 'G': + OPTION('G', "collect", NULL, "Unload unit after it ran, even when failed"): arg_aggressive_gc = true; break; - case 'S': + OPTION('S', "shell", NULL, "Invoke a $SHELL interactively"): arg_shell = true; break; - case ARG_JOB_MODE: - if (streq(optarg, "help")) + OPTION_LONG("job-mode", "MODE", + "Specify how to deal with already queued jobs, when queueing a new job"): + if (streq(opts.arg, "help")) return DUMP_STRING_TABLE(job_mode, JobMode, _JOB_MODE_MAX); - r = job_mode_from_string(optarg); + r = job_mode_from_string(opts.arg); if (r < 0) - return log_error_errno(r, "Invalid job mode: %s", optarg); + return log_error_errno(r, "Invalid job mode: %s", opts.arg); arg_job_mode = r; break; - case ARG_IGNORE_FAILURE: + OPTION_LONG("ignore-failure", NULL, "Ignore the exit status of the invoked process"): arg_ignore_failure = true; break; - case ARG_BACKGROUND: - r = parse_background_argument(optarg, &arg_background); + OPTION_LONG("background", "COLOR", "Set ANSI color for background"): + r = parse_background_argument(opts.arg, &arg_background); if (r < 0) return r; break; - case ARG_NO_PAGER: + OPTION_COMMON_NO_PAGER: arg_pager_flags |= PAGER_DISABLE; break; - case ARG_PATH_PROPERTY: + OPTION_GROUP("Path options"): {} - if (strv_extend(&arg_path_property, optarg) < 0) + OPTION_LONG("path-property", "NAME=VALUE", "Set path unit property"): + if (strv_extend(&arg_path_property, opts.arg) < 0) return log_oom(); - break; - case ARG_SOCKET_PROPERTY: + OPTION_GROUP("Socket options"): {} - if (strv_extend(&arg_socket_property, optarg) < 0) + OPTION_LONG("socket-property", "NAME=VALUE", "Set socket unit property"): + if (strv_extend(&arg_socket_property, opts.arg) < 0) return log_oom(); - break; - case ARG_ON_ACTIVE: - r = add_timer_property("OnActiveSec", optarg); + OPTION_GROUP("Timer options"): {} + + OPTION_LONG("on-active", "SECONDS", "Run after SECONDS delay"): + r = add_timer_property("OnActiveSec", opts.arg); if (r < 0) return r; arg_with_timer = true; break; - case ARG_ON_BOOT: - r = add_timer_property("OnBootSec", optarg); + OPTION_LONG("on-boot", "SECONDS", "Run SECONDS after machine was booted up"): + r = add_timer_property("OnBootSec", opts.arg); if (r < 0) return r; arg_with_timer = true; break; - case ARG_ON_STARTUP: - r = add_timer_property("OnStartupSec", optarg); + OPTION_LONG("on-startup", "SECONDS", "Run SECONDS after systemd activation"): + r = add_timer_property("OnStartupSec", opts.arg); if (r < 0) return r; arg_with_timer = true; break; - case ARG_ON_UNIT_ACTIVE: - r = add_timer_property("OnUnitActiveSec", optarg); + OPTION_LONG("on-unit-active", "SECONDS", "Run SECONDS after the last activation"): + r = add_timer_property("OnUnitActiveSec", opts.arg); if (r < 0) return r; arg_with_timer = true; break; - case ARG_ON_UNIT_INACTIVE: - r = add_timer_property("OnUnitInactiveSec", optarg); + OPTION_LONG("on-unit-inactive", "SECONDS", + "Run SECONDS after the last deactivation"): + r = add_timer_property("OnUnitInactiveSec", opts.arg); if (r < 0) return r; arg_with_timer = true; break; - case ARG_ON_CALENDAR: { + OPTION_LONG("on-calendar", "SPEC", "Realtime timer"): { _cleanup_(calendar_spec_freep) CalendarSpec *cs = NULL; - r = calendar_spec_from_string(optarg, &cs); + r = calendar_spec_from_string(opts.arg, &cs); if (r < 0) return log_error_errno(r, "Failed to parse calendar event specification: %m"); @@ -713,7 +586,7 @@ static int parse_argv(int argc, char *argv[]) { else if (r < 0) return log_error_errno(r, "Failed to calculate next time calendar expression elapses: %m"); - r = add_timer_property("OnCalendar", optarg); + r = add_timer_property("OnCalendar", opts.arg); if (r < 0) return r; @@ -721,7 +594,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - case ARG_ON_TIMEZONE_CHANGE: + OPTION_LONG("on-timezone-change", NULL, "Run when the timezone changes"): r = add_timer_property("OnTimezoneChange", "yes"); if (r < 0) return r; @@ -729,7 +602,7 @@ static int parse_argv(int argc, char *argv[]) { arg_with_timer = true; break; - case ARG_ON_CLOCK_CHANGE: + OPTION_LONG("on-clock-change", NULL, "Run when the realtime clock jumps"): r = add_timer_property("OnClockChange", "yes"); if (r < 0) return r; @@ -737,13 +610,12 @@ static int parse_argv(int argc, char *argv[]) { arg_with_timer = true; break; - case ARG_TIMER_PROPERTY: - - if (strv_extend(&arg_timer_property, optarg) < 0) + OPTION_LONG("timer-property", "NAME=VALUE", "Set timer unit property"): + if (strv_extend(&arg_timer_property, opts.arg) < 0) return log_oom(); arg_with_timer = arg_with_timer || - STARTSWITH_SET(optarg, + STARTSWITH_SET(opts.arg, "OnActiveSec=", "OnBootSec=", "OnStartupSec=", @@ -751,12 +623,6 @@ static int parse_argv(int argc, char *argv[]) { "OnUnitInactiveSec=", "OnCalendar="); break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } /* If we are talking to the per-user instance PolicyKit isn't going to help */ @@ -805,13 +671,14 @@ static int parse_argv(int argc, char *argv[]) { if (arg_pty_late < 0) arg_pty_late = false; /* For systemd-run this defaults to false, for compat reasons */ - if (argc > optind) { + char **args = option_parser_get_args(&opts); + if (!strv_isempty(args)) { char **l; if (arg_shell) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "If --shell is used, no command line is expected."); - l = strv_copy(argv + optind); + l = strv_copy(args); if (!l) return log_oom(); From 03a58fe2517a8015ba83eb9db87e69d60f9a0438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Apr 2026 00:30:08 +0200 Subject: [PATCH 7/9] run: convert run0 option parser to macros This exercises the new option namespace code. --help output is generally the same, except for the formatting changes related to use of the new output helpers and the common option macros. --same-root-dir is now documented (followup for 475729b80532dfbbce98705dade6570ce5cc29f0). Co-developed-by: Claude Opus 4.7 --- src/run/run.c | 227 ++++++++++++++++---------------------------------- 1 file changed, 71 insertions(+), 156 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 36ebe32f2d..885ad23b15 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#include #include #include #include @@ -182,51 +181,27 @@ static int help(void) { } static int help_sudo_mode(void) { - _cleanup_free_ char *link = NULL; + _cleanup_(table_unrefp) Table *opts_table = NULL; int r; - r = terminal_urlify_man("run0", "1", &link); - if (r < 0) - return log_oom(); - /* NB: Let's not go overboard with short options: we try to keep a modicum of compatibility with * sudo's short switches, hence please do not introduce new short switches unless they have a roughly * equivalent purpose on sudo. Use long options for everything private to run0. */ - printf("%s [OPTIONS...] COMMAND [ARGUMENTS...]\n" - "\n%sElevate privileges interactively.%s\n\n" - " -h --help Show this help\n" - " -V --version Show package version\n" - " --no-ask-password Do not prompt for password\n" - " --machine=CONTAINER Operate on local container\n" - " --unit=UNIT Run under the specified unit name\n" - " --property=NAME=VALUE Set service or scope unit property\n" - " --description=TEXT Description for unit\n" - " --slice=SLICE Run in the specified slice\n" - " --slice-inherit Inherit the slice\n" - " -u --user=USER Run as system user\n" - " -g --group=GROUP Run as system group\n" - " --nice=NICE Nice level\n" - " -D --chdir=PATH Set working directory\n" - " --via-shell Invoke command via target user's login shell\n" - " -i Shortcut for --via-shell --chdir='~'\n" - " --setenv=NAME[=VALUE] Set environment variable\n" - " --background=COLOR Set ANSI color for background\n" - " --pty Request allocation of a pseudo TTY for stdio\n" - " --pty-late Just like --pty, but leave TTY access to agents\n" - " until unit is started up\n" - " --pipe Request direct pipe for stdio\n" - " --shell-prompt-prefix=PREFIX Set $SHELL_PROMPT_PREFIX\n" - " --lightweight=BOOLEAN Control whether to register a session with service manager\n" - " or without\n" - " --area=AREA Home area to log into\n" - " --empower Give privileges to selected or current user\n" - "\nSee the %s for details.\n", - program_invocation_short_name, - ansi_highlight(), - ansi_normal(), - link); + r = option_parser_get_help_table_full("run0", /* group= */ NULL, &opts_table); + if (r < 0) + return r; + help_cmdline("[OPTIONS...] COMMAND [ARGUMENTS...]"); + help_abstract("Elevate privileges interactively."); + + help_section("Options:"); + + r = table_print_or_warn(opts_table); + if (r < 0) + return r; + + help_man_page_reference("run0", "1"); return 0; } @@ -795,149 +770,96 @@ static Glyph pty_window_glyph(void) { } static int parse_argv_sudo_mode(int argc, char *argv[]) { - - enum { - ARG_NO_ASK_PASSWORD = 0x100, - ARG_MACHINE, - ARG_UNIT, - ARG_PROPERTY, - ARG_DESCRIPTION, - ARG_SLICE, - ARG_SLICE_INHERIT, - ARG_NICE, - ARG_SETENV, - ARG_BACKGROUND, - ARG_PTY, - ARG_PTY_LATE, - ARG_PIPE, - ARG_SHELL_PROMPT_PREFIX, - ARG_LIGHTWEIGHT, - ARG_AREA, - ARG_VIA_SHELL, - ARG_EMPOWER, - ARG_SAME_ROOT_DIR, - }; + int r; /* If invoked as "run0" binary, let's expose a more sudo-like interface. We add various extensions * though (but limit the extension to long options). */ - static const struct option options[] = { - { "help", no_argument, NULL, 'h' }, - { "version", no_argument, NULL, 'V' }, - { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, - { "machine", required_argument, NULL, ARG_MACHINE }, - { "unit", required_argument, NULL, ARG_UNIT }, - { "property", required_argument, NULL, ARG_PROPERTY }, - { "description", required_argument, NULL, ARG_DESCRIPTION }, - { "slice", required_argument, NULL, ARG_SLICE }, - { "slice-inherit", no_argument, NULL, ARG_SLICE_INHERIT }, - { "user", required_argument, NULL, 'u' }, - { "group", required_argument, NULL, 'g' }, - { "nice", required_argument, NULL, ARG_NICE }, - { "chdir", required_argument, NULL, 'D' }, - { "via-shell", no_argument, NULL, ARG_VIA_SHELL }, - { "login", no_argument, NULL, 'i' }, /* compat with sudo, --via-shell + --chdir='~' */ - { "setenv", required_argument, NULL, ARG_SETENV }, - { "background", required_argument, NULL, ARG_BACKGROUND }, - { "pty", no_argument, NULL, ARG_PTY }, - { "pty-late", no_argument, NULL, ARG_PTY_LATE }, - { "pipe", no_argument, NULL, ARG_PIPE }, - { "shell-prompt-prefix", required_argument, NULL, ARG_SHELL_PROMPT_PREFIX }, - { "lightweight", required_argument, NULL, ARG_LIGHTWEIGHT }, - { "area", required_argument, NULL, ARG_AREA }, - { "empower", no_argument, NULL, ARG_EMPOWER }, - { "same-root-dir", no_argument, NULL, ARG_SAME_ROOT_DIR }, - {}, - }; - - int r, c; - assert(argc >= 0); assert(argv); - /* Resetting to 0 forces the invocation of an internal initialization routine of getopt_long() - * that checks for GNU extensions in optstring ('-' or '+' at the beginning). */ - optind = 0; - while ((c = getopt_long(argc, argv, "+hVu:g:D:i", options, NULL)) >= 0) + OptionParser opts = { argc, argv, OPTION_PARSER_STOP_AT_FIRST_NONOPTION, "run0" }; + FOREACH_OPTION(c, &opts, /* on_error= */ return c) switch (c) { - case 'h': + OPTION_NAMESPACE("run0"): {} + + OPTION_COMMON_HELP: return help_sudo_mode(); - case 'V': + OPTION('V', "version", NULL, "Show package version"): return version(); - case ARG_NO_ASK_PASSWORD: + OPTION_COMMON_NO_ASK_PASSWORD: arg_ask_password = false; break; - case ARG_MACHINE: - r = parse_machine_argument(optarg, &arg_host, &arg_transport); + OPTION_LONG("machine", "CONTAINER", "Operate on local container"): + r = parse_machine_argument(opts.arg, &arg_host, &arg_transport); if (r < 0) return r; break; - case ARG_UNIT: - arg_unit = optarg; + OPTION_LONG("unit", "UNIT", "Run under the specified unit name"): + arg_unit = opts.arg; break; - case ARG_PROPERTY: - if (strv_extend(&arg_property, optarg) < 0) + OPTION_LONG("property", "NAME=VALUE", "Set service or scope unit property"): + if (strv_extend(&arg_property, opts.arg) < 0) return log_oom(); - break; - case ARG_DESCRIPTION: - r = free_and_strdup_warn(&arg_description, optarg); + OPTION_LONG("description", "TEXT", "Description for unit"): + r = free_and_strdup_warn(&arg_description, opts.arg); if (r < 0) return r; break; - case ARG_SLICE: - r = free_and_strdup_warn(&arg_slice, optarg); + OPTION_LONG("slice", "SLICE", "Run in the specified slice"): + r = free_and_strdup_warn(&arg_slice, opts.arg); if (r < 0) return r; break; - case ARG_SLICE_INHERIT: + OPTION_LONG("slice-inherit", NULL, "Inherit the slice"): arg_slice_inherit = true; break; - case 'u': - r = free_and_strdup_warn(&arg_exec_user, optarg); + OPTION('u', "user", "USER", "Run as system user"): + r = free_and_strdup_warn(&arg_exec_user, opts.arg); if (r < 0) return r; break; - case 'g': - arg_exec_group = optarg; + OPTION('g', "group", "GROUP", "Run as system group"): + arg_exec_group = opts.arg; break; - case ARG_NICE: - r = parse_nice(optarg, &arg_nice); + OPTION_LONG("nice", "NICE", "Nice level"): + r = parse_nice(opts.arg, &arg_nice); if (r < 0) - return log_error_errno(r, "Failed to parse nice value: %s", optarg); + return log_error_errno(r, "Failed to parse nice value: %s", opts.arg); arg_nice_set = true; break; - case 'D': - if (streq(optarg, "~")) - r = free_and_strdup_warn(&arg_working_directory, optarg); + OPTION('D', "chdir", "PATH", "Set working directory"): + if (streq(opts.arg, "~")) + r = free_and_strdup_warn(&arg_working_directory, opts.arg); else /* Root will be manually suppressed later. */ - r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_working_directory); + r = parse_path_argument(opts.arg, /* suppress_root= */ false, &arg_working_directory); if (r < 0) return r; - break; - case ARG_VIA_SHELL: + OPTION_LONG("via-shell", NULL, "Invoke command via target user's login shell"): arg_via_shell = true; break; - case 'i': + OPTION_LONG("login", NULL, NULL): {} /* hidden compat alias for -i */ + OPTION_SHORT('i', NULL, "Shortcut for --via-shell --chdir='~'"): r = free_and_strdup_warn(&arg_working_directory, "~"); if (r < 0) return r; @@ -945,69 +867,61 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { arg_via_shell = true; break; - case ARG_SETENV: - r = strv_env_replace_strdup_passthrough(&arg_environment, optarg); + OPTION_LONG("setenv", "NAME[=VALUE]", "Set environment variable"): + r = strv_env_replace_strdup_passthrough(&arg_environment, opts.arg); if (r < 0) - return log_error_errno(r, "Cannot assign environment variable %s: %m", optarg); - + return log_error_errno(r, "Cannot assign environment variable %s: %m", opts.arg); break; - case ARG_BACKGROUND: - r = parse_background_argument(optarg, &arg_background); + OPTION_LONG("background", "COLOR", "Set ANSI color for background"): + r = parse_background_argument(opts.arg, &arg_background); if (r < 0) return r; - break; - case ARG_PTY: - case ARG_PTY_LATE: + OPTION_LONG("pty", NULL, "Request allocation of a pseudo TTY for stdio"): {} + OPTION_LONG("pty-late", NULL, + "Just like --pty, but leave TTY access to agents until unit is started up"): arg_stdio |= ARG_STDIO_PTY; - arg_pty_late = c == ARG_PTY_LATE; + arg_pty_late = streq(opts.opt->long_code, "pty-late"); break; - case ARG_PIPE: + OPTION_LONG("pipe", NULL, "Request direct pipe for stdio"): arg_stdio |= ARG_STDIO_DIRECT; break; - case ARG_SHELL_PROMPT_PREFIX: - r = free_and_strdup_warn(&arg_shell_prompt_prefix, optarg); + OPTION_LONG("shell-prompt-prefix", "PREFIX", "Set $SHELL_PROMPT_PREFIX"): + r = free_and_strdup_warn(&arg_shell_prompt_prefix, opts.arg); if (r < 0) return r; break; - case ARG_LIGHTWEIGHT: - r = parse_tristate_argument_with_auto("--lightweight=", optarg, &arg_lightweight); + OPTION_LONG("lightweight", "BOOLEAN", + "Control whether to register a session with service manager or without"): + r = parse_tristate_argument_with_auto("--lightweight=", opts.arg, &arg_lightweight); if (r < 0) return r; break; - case ARG_AREA: + OPTION_LONG("area", "AREA", "Home area to log into"): /* We allow an empty --area= specification to allow logging into the primary home directory */ - if (!isempty(optarg) && !filename_is_valid(optarg)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid area name, refusing: %s", optarg); + if (!isempty(opts.arg) && !filename_is_valid(opts.arg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid area name, refusing: %s", opts.arg); - r = free_and_strdup_warn(&arg_area, optarg); + r = free_and_strdup_warn(&arg_area, opts.arg); if (r < 0) return r; - break; - case ARG_EMPOWER: + OPTION_LONG("empower", NULL, "Give privileges to selected or current user"): arg_empower = true; break; - case ARG_SAME_ROOT_DIR: + OPTION_LONG("same-root-dir", NULL, NULL): /* hidden */ r = free_and_strdup_warn(&arg_root_directory, "/"); if (r < 0) return r; - break; - - case '?': - return -EINVAL; - - default: - assert_not_reached(); } if (!arg_working_directory) { @@ -1055,8 +969,9 @@ static int parse_argv_sudo_mode(int argc, char *argv[]) { arg_send_sighup = true; _cleanup_strv_free_ char **l = NULL; - if (argc > optind) { - l = strv_copy(argv + optind); + char **args = option_parser_get_args(&opts); + if (!strv_isempty(args)) { + l = strv_copy(args); if (!l) return log_oom(); } else if (!arg_via_shell) { From 0a85be9ccf1bdd0fb19caf1178958f243c6fc9ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Apr 2026 08:11:40 +0200 Subject: [PATCH 8/9] run: use a "named namespace" also for the main option parser It seems that clang reorders the entries in the options array that originate from different functions, but not within a function. Using "named namespaces" exclusively should sidestep the issue. (A bigger hammer would be to sort the array. We *can* do this, since the options have the increasing .id field. But that'd require duplicating the memory or making it writable. Let's avoid this until we know for sure that it's needed.) --- src/run/run.c | 6 ++++-- src/shared/options.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/run/run.c b/src/run/run.c index 885ad23b15..ce35b48fba 100644 --- a/src/run/run.c +++ b/src/run/run.c @@ -154,7 +154,7 @@ static int help(void) { _cleanup_(table_unref_many) Table *tables[ELEMENTSOF(groups) + 1] = {}; for (size_t i = 0; i < ELEMENTSOF(groups); i++) { - r = option_parser_get_help_table_group(groups[i], &tables[i]); + r = option_parser_get_help_table_full("systemd-run", groups[i], &tables[i]); if (r < 0) return r; } @@ -252,11 +252,13 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser opts = { argc, argv, OPTION_PARSER_STOP_AT_FIRST_NONOPTION }; + OptionParser opts = { argc, argv, OPTION_PARSER_STOP_AT_FIRST_NONOPTION, "systemd-run" }; FOREACH_OPTION(c, &opts, /* on_error= */ return c) switch (c) { + OPTION_NAMESPACE("systemd-run"): {} + OPTION_COMMON_HELP: return help(); diff --git a/src/shared/options.h b/src/shared/options.h index 7d8507d0e5..8b8fc9ed4a 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -9,6 +9,8 @@ * By default, options defined in a single program are all placed in a single (unnamed) namespace * and in a single (unnamed) group. OPTION_NAMESPACE() marks the beginning of a named namespace. * OPTION_GROUP() marks the beginning of a named group. + * + * Note: if multiple namespaces are used, they should all be named. */ typedef enum OptionFlags { From 4c640beba6faf717d7f145036af4fa5ba430e6a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 28 Apr 2026 08:45:03 +0200 Subject: [PATCH 9/9] shared/options: add an assert loop to verify ordering If things are misordered, we need to catch this. Use assert_se to make the check also in custom builds that otherwise disable assertions. --- src/shared/options.c | 6 ++++++ src/shared/options.h | 9 ++++++++- src/test/test-options.c | 18 +++++++++--------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/shared/options.c b/src/shared/options.c index 364b57792c..227ba8ffa3 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -101,6 +101,12 @@ int option_parse( state->namespace_start = options; const Option *opt; + + /* Verify that the option array didn't get mangled within a namespace. */ + for (opt = options; opt < options_end; opt++) + if (opt + 1 < options_end && !FLAGS_SET((opt + 1)->flags, OPTION_NAMESPACE_MARKER)) + assert_se(opt->id < (opt + 1)->id); + for (opt = options; opt < options_end; opt++) { bool ns_marker = FLAGS_SET(opt->flags, OPTION_NAMESPACE_MARKER); if (!in_ns) { diff --git a/src/shared/options.h b/src/shared/options.h index 8b8fc9ed4a..7bb73dd581 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -10,7 +10,14 @@ * and in a single (unnamed) group. OPTION_NAMESPACE() marks the beginning of a named namespace. * OPTION_GROUP() marks the beginning of a named group. * - * Note: if multiple namespaces are used, they should all be named. + * Note: if multiple namespaces are used, they should all be named, i.e. each separate parse_argv + * instance should have OPTION_NAMESPACE first, and then its set of OPTION()s. (This is because + * clang reorders OPTIONs coming from different functions. So an unnamed group could end up being + * merged with one of the earlier groups. It seems that reordering within a single function does + * not happen.) + * + * When groups are used, the first group may be named (with OPTION_GROUP appearing before any + * options), or it may be unnamed. Both variants should work fine. */ typedef enum OptionFlags { diff --git a/src/test/test-options.c b/src/test/test-options.c index a77964df4e..150c2b2a56 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -631,12 +631,12 @@ TEST(option_stops_parsing) { 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" }, + { __COUNTER__, .short_code = 'h', .long_code = "help" }, + { __COUNTER__, .long_code = "version" }, + { __COUNTER__, .long_code = "AdvancedGroup", .flags = OPTION_GROUP_MARKER }, + { __COUNTER__, .long_code = "debug" }, + { __COUNTER__, .long_code = "Advance" }, /* prefix match with the group */ + { __COUNTER__, .long_code = "defilbrilate" }, {} }; @@ -1297,9 +1297,9 @@ TEST(option_macros) { * peeks at the next arg to handle legacy "space-separated" form. */ TEST(option_optional_arg_consume) { static const Option options[] = { - { 1, .short_code = 'h', .long_code = "help" }, - { 2, .long_code = "user", .metavar = "NAME", .flags = OPTION_OPTIONAL_ARG }, - { 3, .short_code = 'u', .long_code = "uid", .metavar = "USER" }, + { __COUNTER__, .short_code = 'h', .long_code = "help" }, + { __COUNTER__, .long_code = "user", .metavar = "NAME", .flags = OPTION_OPTIONAL_ARG }, + { __COUNTER__, .short_code = 'u', .long_code = "uid", .metavar = "USER" }, {} };