From: Lennart Poettering Date: Fri, 24 Apr 2026 08:33:46 +0000 (+0200) Subject: options: add a proper state machine field X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ce6a5f416432a86c702912a0edf18c2ca34a1314;p=thirdparty%2Fsystemd.git 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. --- diff --git a/src/shared/options.c b/src/shared/options.c index 0c3188bf786..faefbe659ea 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 889867bf6a9..81b92d2da5f 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.