SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

options: add a proper state machine field

Browse Source 
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.
This commit is contained in:
Lennart Poettering
2026-04-27 20:21:04 +02:00
committed by Zbigniew Jędrzejewski-Szmek
parent bd2d2fec80
commit ce6a5f4164
2 changed files with 95 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/options.c
+83 -39
View File
@@ -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;
src/shared/options.h
+12 -2
View File
@@ -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.