From f4a5bf92e9d3860958ef1aecd2208f81c3dcbd90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 25 Mar 2026 16:35:12 +0100 Subject: [PATCH] shared/options: store argc+argv in the OptionParser state struct After writing the code to parse options in a bunch of places, I think passing the argc+argv to various functions to query state is annoying. It seems nicer to just stash them in the state struct once. --- src/ac-power/ac-power.c | 6 ++-- src/ask-password/ask-password.c | 6 ++-- src/binfmt/binfmt.c | 6 ++-- src/bless-boot/bless-boot.c | 6 ++-- src/dissect/dissect.c | 6 ++-- src/id128/id128.c | 6 ++-- src/notify/notify.c | 6 ++-- src/shared/options.c | 56 +++++++++++++++------------------ src/shared/options.h | 17 +++++----- src/test/test-options.c | 18 +++++------ src/update-done/update-done.c | 6 ++-- src/validatefs/validatefs.c | 6 ++-- 12 files changed, 72 insertions(+), 73 deletions(-) diff --git a/src/ac-power/ac-power.c b/src/ac-power/ac-power.c index e6b58810a6..1ca1048c5a 100644 --- a/src/ac-power/ac-power.c +++ b/src/ac-power/ac-power.c @@ -48,10 +48,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -68,7 +68,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - if (option_parser_get_n_args(&state, argc) > 0) + if (option_parser_get_n_args(&state) > 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program takes no arguments."); return 1; diff --git a/src/ask-password/ask-password.c b/src/ask-password/ask-password.c index 13e2368795..2c032c1afb 100644 --- a/src/ask-password/ask-password.c +++ b/src/ask-password/ask-password.c @@ -71,10 +71,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -170,7 +170,7 @@ static int parse_argv(int argc, char *argv[]) { SET_FLAG(arg_flags, ASK_PASSWORD_HIDE_EMOJI, !r); } - char **args = option_parser_get_args(&state, argc, argv); + char **args = option_parser_get_args(&state); if (!strv_isempty(args)) { arg_message = strv_join(args, " "); diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c index bdd62398e5..23c09fe349 100644 --- a/src/binfmt/binfmt.c +++ b/src/binfmt/binfmt.c @@ -137,10 +137,10 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -165,7 +165,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - char **args = option_parser_get_args(&state, argc, argv); + char **args = option_parser_get_args(&state); if ((arg_unregister || arg_cat_flags != CAT_CONFIG_OFF) && !strv_isempty(args)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c index daabff405f..bf1c6e7a0c 100644 --- a/src/bless-boot/bless-boot.c +++ b/src/bless-boot/bless-boot.c @@ -75,10 +75,10 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -93,7 +93,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - *ret_args = option_parser_get_args(&state, argc, argv); + *ret_args = option_parser_get_args(&state); return 1; } diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index d36f31e5c7..3597971af9 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -207,11 +207,11 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const Option *current; const char *arg; - FOREACH_OPTION_FULL(&state, c, argc, argv, ¤t, &arg, /* on_error= */ return c) + FOREACH_OPTION_FULL(&state, c, ¤t, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_NO_PAGER: @@ -495,7 +495,7 @@ static int parse_argv(int argc, char *argv[]) { 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); + char **args = option_parser_get_args(&state); switch (arg_action) { diff --git a/src/id128/id128.c b/src/id128/id128.c index ebed02913f..688504c714 100644 --- a/src/id128/id128.c +++ b/src/id128/id128.c @@ -233,10 +233,10 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -288,7 +288,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { break; } - *ret_args = option_parser_get_args(&state, argc, argv); + *ret_args = option_parser_get_args(&state); return 1; } diff --git a/src/notify/notify.c b/src/notify/notify.c index 5535296760..a06f5ce773 100644 --- a/src/notify/notify.c +++ b/src/notify/notify.c @@ -158,10 +158,10 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: @@ -283,7 +283,7 @@ static int parse_argv(int argc, char *argv[], char ***ret_args) { 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); + char **args = option_parser_get_args(&state); switch (arg_action) { diff --git a/src/shared/options.c b/src/shared/options.c index 508db28ea0..86f274821e 100644 --- a/src/shared/options.c +++ b/src/shared/options.c @@ -69,7 +69,6 @@ int option_parse( const Option options[], const Option options_end[], OptionParser *state, - int argc, char *argv[], const Option **ret_option, const char **ret_arg) { @@ -77,13 +76,10 @@ int option_parse( /* Check and initialize */ if (state->optind == 0) { - if (argc < 1 || strv_isempty(argv)) + if (state->argc < 1 || strv_isempty(state->argv)) return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "argv cannot be empty"); - *state = (OptionParser) { - .optind = 1, - .positional_offset = 1, - }; + state->optind = state->positional_offset = 1; } /* Look for the next option */ @@ -96,21 +92,21 @@ int option_parse( if (state->short_option_offset == 0) { /* Skip over non-option parameters */ for (;;) { - if (state->optind == argc) + if (state->optind == state->argc) return 0; - if (streq(argv[state->optind], "--")) { + if (streq(state->argv[state->optind], "--")) { /* No more options. Move "--" before positional args so that * the list of positional args is clean. */ - shift_arg(argv, state->positional_offset++, state->optind++); + shift_arg(state->argv, state->positional_offset++, state->optind++); state->parsing_stopped = true; } if (state->parsing_stopped) return 0; - if (argv[state->optind][0] == '-' && - argv[state->optind][1] != '\0') + if (state->argv[state->optind][0] == '-' && + state->argv[state->optind][1] != '\0') /* Looks like we found an option parameter */ break; @@ -119,13 +115,13 @@ int option_parse( /* Find matching option entry. * First, figure out if we have a long option or a short option. */ - assert(argv[state->optind][0] == '-'); + assert(state->argv[state->optind][0] == '-'); - if (argv[state->optind][1] == '-') { + if (state->argv[state->optind][1] == '-') { /* We have a long option. */ - char *eq = strchr(argv[state->optind], '='); + char *eq = strchr(state->argv[state->optind], '='); if (eq) { - optname = _optname = strndup(argv[state->optind], eq - argv[state->optind]); + optname = _optname = strndup(state->argv[state->optind], eq - state->argv[state->optind]); if (!_optname) return log_oom(); @@ -133,7 +129,7 @@ int option_parse( optval = eq + 1; } else /* argument (if any) is separate */ - optname = argv[state->optind]; + optname = state->argv[state->optind]; const Option *last_partial = NULL; unsigned n_partial_matches = 0; /* The commandline option matches a defined prefix. */ @@ -172,7 +168,7 @@ int option_parse( } if (state->short_option_offset > 0) { - char optchar = argv[state->optind][state->short_option_offset]; + char optchar = state->argv[state->optind][state->short_option_offset]; if (asprintf(&_optname, "-%c", optchar) < 0) return log_oom(); @@ -187,7 +183,7 @@ int option_parse( if (option_is_metadata(option) || optchar != option->short_code) continue; - const char *rest = argv[state->optind] + state->short_option_offset + 1; + const char *rest = state->argv[state->optind] + state->short_option_offset + 1; if (option_takes_arg(option) && !isempty(rest)) { /* The rest of this parameter is the value. */ @@ -209,19 +205,19 @@ int option_parse( "%s: option '%s' doesn't allow an argument", program_invocation_short_name, optname); if (!optval && option_arg_required(option)) { - if (!argv[state->optind + 1]) + if (!state->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]; + optval = state->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++); + shift_arg(state->argv, state->positional_offset++, state->optind++); if (separate_optval) - shift_arg(argv, state->positional_offset++, state->optind++); + shift_arg(state->argv, state->positional_offset++, state->optind++); } if (FLAGS_SET(option->flags, OPTION_STOPS_PARSING)) @@ -234,25 +230,25 @@ int option_parse( return option->id; } -char** option_parser_get_args(const OptionParser *state, int argc, char *argv[]) { +char** option_parser_get_args(const OptionParser *state) { /* Returns positional args as a strv. * If "--" was found, it has been moved before state->positional_offset. * The array is only valid, i.e. clean without any options, after parsing * has naturally finished. */ assert(state->optind > 0); - assert(state->optind == argc || state->parsing_stopped); - assert(state->positional_offset <= argc); + assert(state->optind == state->argc || state->parsing_stopped); + assert(state->positional_offset <= state->argc); - return argv + state->positional_offset; + return state->argv + state->positional_offset; } -size_t option_parser_get_n_args(const OptionParser *state, int argc) { +size_t option_parser_get_n_args(const OptionParser *state) { assert(state->optind > 0); - assert(state->optind == argc || state->parsing_stopped); - assert(state->positional_offset <= argc); + assert(state->optind == state->argc || state->parsing_stopped); + assert(state->positional_offset <= state->argc); - return argc - state->positional_offset; + return state->argc - state->positional_offset; } int _option_parser_get_help_table( diff --git a/src/shared/options.h b/src/shared/options.h index 4895c10cbd..b86a728794 100644 --- a/src/shared/options.h +++ b/src/shared/options.h @@ -68,6 +68,10 @@ extern const Option __start_SYSTEMD_OPTIONS[]; extern const Option __stop_SYSTEMD_OPTIONS[]; typedef struct OptionParser { + /* Those two should stay first so that it's possible to initialize the struct as { argc, argv }. */ + int argc; /* The original argc. */ + char **argv; /* The argv array, possibly reordered. */ + 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. @@ -81,23 +85,22 @@ 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; ) \ +#define FOREACH_OPTION_FULL(parser, opt, ret_o, ret_a, on_error) \ + for (int opt; (opt = option_parse(ALIGN_PTR(__start_SYSTEMD_OPTIONS), __stop_SYSTEMD_OPTIONS, parser, 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) +#define FOREACH_OPTION(parser, opt, ret_a, on_error) \ + FOREACH_OPTION_FULL(parser, opt, /* ret_o= */ NULL, ret_a, on_error) -char** option_parser_get_args(const OptionParser *state, int argc, char *argv[]); -size_t option_parser_get_n_args(const OptionParser *state, int argc); +char** option_parser_get_args(const OptionParser *state); +size_t option_parser_get_n_args(const OptionParser *state); int _option_parser_get_help_table( const Option options[], diff --git a/src/test/test-options.c b/src/test/test-options.c index e849365a2f..fb1f61f358 100644 --- a/src/test/test-options.c +++ b/src/test/test-options.c @@ -31,10 +31,10 @@ static void test_option_parse_one( for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) n_entries++; - OptionParser state = {}; + OptionParser state = { argc, argv }; const Option *opt; const char *arg; - for (int c; (c = option_parse(options, options + n_options, &state, argc, argv, &opt, &arg)) != 0; ) { + for (int c; (c = option_parse(options, options + n_options, &state, &opt, &arg)) != 0; ) { ASSERT_OK(c); ASSERT_NOT_NULL(opt); @@ -54,11 +54,11 @@ static void test_option_parse_one( ASSERT_EQ(i, n_entries); - char **args = option_parser_get_args(&state, argc, argv); + char **args = option_parser_get_args(&state); ASSERT_TRUE(strv_equal(args, remaining)); ASSERT_STREQ(argv[0], saved_argv0); - ASSERT_EQ(option_parser_get_n_args(&state, argc), strv_length(remaining)); + ASSERT_EQ(option_parser_get_n_args(&state), strv_length(remaining)); } static void test_option_invalid_one( @@ -77,11 +77,11 @@ static void test_option_invalid_one( for (const Option *o = options; o->short_code != 0 || o->long_code; o++) n_options++; - OptionParser state = {}; + OptionParser state = { argc, argv }; const Option *opt; const char *arg; - int c = option_parse(options, options + n_options, &state, argc, argv, &opt, &arg); + int c = option_parse(options, options + n_options, &state, &opt, &arg); ASSERT_ERROR(c, EINVAL); } @@ -691,11 +691,11 @@ static void test_macros_parse_one( for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++) n_entries++; - OptionParser state = {}; + OptionParser state = { argc, argv }; const Option *opt; const char *arg; - FOREACH_OPTION_FULL(&state, c, argc, argv, &opt, &arg, ASSERT_TRUE(false)) { + FOREACH_OPTION_FULL(&state, c, &opt, &arg, ASSERT_TRUE(false)) { log_debug("%c %s: %s=%s", opt->short_code != 0 ? opt->short_code : ' ', opt->long_code ?: "", @@ -751,7 +751,7 @@ static void test_macros_parse_one( ASSERT_EQ(i, n_entries); - char **args = option_parser_get_args(&state, argc, argv); + char **args = option_parser_get_args(&state); ASSERT_TRUE(strv_equal(args, remaining)); ASSERT_STREQ(argv[0], saved_argv0); } diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c index 7fb1c58d19..b3c45c352b 100644 --- a/src/update-done/update-done.c +++ b/src/update-done/update-done.c @@ -94,10 +94,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -112,7 +112,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - if (option_parser_get_n_args(&state, argc) > 0) + if (option_parser_get_n_args(&state) > 0) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program takes no arguments."); return 1; diff --git a/src/validatefs/validatefs.c b/src/validatefs/validatefs.c index c507e4d98a..9645fd187f 100644 --- a/src/validatefs/validatefs.c +++ b/src/validatefs/validatefs.c @@ -62,10 +62,10 @@ static int parse_argv(int argc, char *argv[]) { assert(argc >= 0); assert(argv); - OptionParser state = {}; + OptionParser state = { argc, argv }; const char *arg; - FOREACH_OPTION(&state, c, argc, argv, &arg, /* on_error= */ return c) + FOREACH_OPTION(&state, c, &arg, /* on_error= */ return c) switch (c) { OPTION_COMMON_HELP: return help(); @@ -88,7 +88,7 @@ static int parse_argv(int argc, char *argv[]) { break; } - char **args = option_parser_get_args(&state, argc, argv); + char **args = option_parser_get_args(&state); if (strv_length(args) != 1) return log_error_errno(SYNTHETIC_ERRNO(EINVAL),