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

Macroize option parsing in nspawn, vmspawn, journal-remote (#41542)

Browse Source
This commit is contained in:
Daan De Meyer
2026-04-08 15:33:51 +02:00
committed by GitHub
parent b5a1ecb8fc e02f66d36f
commit 40d0fb2eb2
5 changed files with 1391 additions and 1574 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/nspawn/nspawn.c
+649 -925
View File
File diff suppressed because it is too large Load Diff
src/shared/options.c
+27
View File
@@ -108,6 +108,11 @@ int option_parse(
/* Looks like we found an option parameter */
break;
if (state->stop_at_first_nonoption) {
state->parsing_stopped = true;
return 0;
}
state->optind++;
}
@@ -234,6 +239,28 @@ int option_parse(
return option->id;
}
char* option_parser_next_arg(const OptionParser *state) {
/* Peek at the next argument, whatever it is (option or position arg).
* May return NULL. */
assert(state->optind > 0);
assert(state->positional_offset <= state->argc);
return state->optind < state->argc ? state->argv[state->optind] : NULL;
}
char* option_parser_consume_next_arg(OptionParser *state) {
/* "Take" the next argument, whatever it is (option or position arg).
* The argument remains in the array, but the optind pointer is moved
* so we won't try to interpret it as an option.
* May return NULL. */
char *t = option_parser_next_arg(state);
if (t)
shift_arg(state->argv, state->positional_offset++, state->optind++);
return t;
}
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.
src/shared/options.h
+16 -10
View File
@@ -79,17 +79,20 @@ 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. */
/* Those three should stay first so that it's possible to initialize the struct as { argc, argv }
* or { argc, argv, true/false }. */
int argc; /* The original argc. */
char **argv; /* The argv array, possibly reordered. */
bool stop_at_first_nonoption; /* Same as "+…" for getopt_long — only parse options before the first
* positional argument. */
int optind; /* Position of the parameter being handled.
* 0 → option parsing hasn't been started yet. */
int short_option_offset; /* Set when we're parsing an argument with one or more short options.
* 0 → we're not parsing short options. */
int positional_offset; /* Offset to where positional parameters are. After processing has been
* finished, all options and their args are to the left of this offset. */
bool parsing_stopped; /* We processed "--" or an option that terminates option parsing. */
bool parsing_stopped; /* We processed "--" or an option that terminates option parsing. */
int optind; /* Position of the parameter being handled.
* 0 → option parsing hasn't been started yet. */
int short_option_offset; /* Set when we're parsing an argument with one or more short options.
* 0 → we're not parsing short options. */
int positional_offset; /* Offset to where positional parameters are. After processing has been
* finished, all options and their args are to the left of this offset. */
} OptionParser;
int option_parse(
@@ -110,6 +113,9 @@ int option_parse(
#define FOREACH_OPTION(parser, opt, ret_a, on_error) \
FOREACH_OPTION_FULL(parser, opt, /* ret_o= */ NULL, ret_a, on_error)
char* option_parser_next_arg(const OptionParser *state);
char* option_parser_consume_next_arg(OptionParser *state);
char** option_parser_get_args(const OptionParser *state);
size_t option_parser_get_n_args(const OptionParser *state);
src/test/test-options.c
+246 -41
View File
@@ -14,7 +14,8 @@ static void test_option_parse_one(
char **argv,
const Option options[],
const Entry *entries,
char **remaining) {
char **remaining,
bool stop_at_first_nonoption) {
_cleanup_free_ char *joined = strv_join(argv, ", ");
log_debug("/* %s(%s) */", __func__, joined);
@@ -31,7 +32,7 @@ static void test_option_parse_one(
for (const Entry *e = entries; e && (e->long_code || e->short_code != 0); e++)
n_entries++;
OptionParser state = { argc, argv };
OptionParser state = { argc, argv, stop_at_first_nonoption };
const Option *opt;
const char *arg;
for (int c; (c = option_parse(options, options + n_options, &state, &opt, &arg)) != 0; ) {
@@ -99,7 +100,8 @@ TEST(option_parse) {
test_option_parse_one(STRV_MAKE("arg0"),
options,
NULL,
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -111,7 +113,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"--",
@@ -124,7 +127,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"--help",
"-h",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -137,7 +141,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"--",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -150,7 +155,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"--help"),
@@ -159,7 +165,21 @@ TEST(option_parse) {
{ "help" },
{}
},
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"--help",
"string1",
"--help"),
options,
(Entry[]) {
{ "help" },
{}
},
STRV_MAKE("string1",
"--help"),
true);
test_option_parse_one(STRV_MAKE("arg0",
"-h"),
@@ -168,7 +188,8 @@ TEST(option_parse) {
{ "help" },
{}
},
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"--help",
@@ -184,7 +205,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"-h",
@@ -200,7 +222,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -216,7 +239,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -232,7 +256,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -248,7 +273,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -264,7 +290,8 @@ TEST(option_parse) {
STRV_MAKE("string1",
"string2",
"string3",
"string4"));
"string4"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"--required1", "reqarg1"),
@@ -273,7 +300,8 @@ TEST(option_parse) {
{ "required1", "reqarg1" },
{}
},
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"-r", "reqarg1"),
@@ -282,7 +310,8 @@ TEST(option_parse) {
{ "required1", "reqarg1" },
{}
},
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -294,7 +323,19 @@ TEST(option_parse) {
{}
},
STRV_MAKE("string1",
"string2"));
"string2"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
"string2",
"-r", "reqarg1"),
options,
NULL,
STRV_MAKE("string1",
"string2",
"-r", "reqarg1"),
true);
test_option_parse_one(STRV_MAKE("arg0",
"--optional1=optarg1"),
@@ -303,7 +344,8 @@ TEST(option_parse) {
{ "optional1", "optarg1" },
{}
},
NULL);
NULL,
true);
test_option_parse_one(STRV_MAKE("arg0",
"--optional1", "string1"),
@@ -312,7 +354,8 @@ TEST(option_parse) {
{ "optional1", NULL },
{}
},
STRV_MAKE("string1"));
STRV_MAKE("string1"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"-ooptarg1"),
@@ -321,7 +364,8 @@ TEST(option_parse) {
{ "optional1", "optarg1" },
{}
},
NULL);
NULL,
false);
test_option_parse_one(STRV_MAKE("arg0",
"-o", "string1"),
@@ -330,7 +374,8 @@ TEST(option_parse) {
{ "optional1", NULL },
{}
},
STRV_MAKE("string1"));
STRV_MAKE("string1"),
false);
test_option_parse_one(STRV_MAKE("arg0",
"string1",
@@ -399,7 +444,8 @@ TEST(option_parse) {
"string7",
"--help",
"--required1",
"--optional1"));
"--optional1"),
false);
}
TEST(option_stops_parsing) {
@@ -422,7 +468,8 @@ TEST(option_stops_parsing) {
{}
},
STRV_MAKE("--help",
"foo"));
"foo"),
false);
/* Options before --exec are still parsed */
test_option_parse_one(STRV_MAKE("arg0",
@@ -437,7 +484,8 @@ TEST(option_stops_parsing) {
{}
},
STRV_MAKE("--version",
"bar"));
"bar"),
false);
/* --exec with no trailing args */
test_option_parse_one(STRV_MAKE("arg0",
@@ -447,7 +495,8 @@ TEST(option_stops_parsing) {
{ "exec" },
{}
},
NULL);
NULL,
false);
/* --exec after positional args */
test_option_parse_one(STRV_MAKE("arg0",
@@ -463,7 +512,8 @@ TEST(option_stops_parsing) {
STRV_MAKE("pos1",
"--help",
"--required",
"val"));
"val"),
false);
/* "--" 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
@@ -478,7 +528,8 @@ TEST(option_stops_parsing) {
{ "exec" },
{}
},
STRV_MAKE("--help"));
STRV_MAKE("--help"),
false);
/* "--" before --exec: "--" terminates first, --exec is positional */
test_option_parse_one(STRV_MAKE("arg0",
@@ -488,7 +539,8 @@ TEST(option_stops_parsing) {
options,
NULL,
STRV_MAKE("--exec",
"--help"));
"--help"),
false);
/* Multiple options then --exec then more option-like args */
test_option_parse_one(STRV_MAKE("arg0",
@@ -506,7 +558,8 @@ TEST(option_stops_parsing) {
},
STRV_MAKE("-h",
"--required",
"val2"));
"val2"),
false);
}
TEST(option_group_marker) {
@@ -530,7 +583,8 @@ TEST(option_group_marker) {
{ "debug" },
{}
},
NULL);
NULL,
false);
/* Check that group marker name is ignored */
test_option_parse_one(STRV_MAKE("arg0",
@@ -542,7 +596,8 @@ TEST(option_group_marker) {
{ "version" },
{}
},
NULL);
NULL,
false);
/* Verify that the group marker is not mistaken for an option */
test_option_invalid_one(STRV_MAKE("arg0",
@@ -569,7 +624,8 @@ TEST(option_group_marker) {
{ "Advance" },
{}
},
NULL);
NULL,
false);
/* Partial match with multiple candidates */
test_option_invalid_one(STRV_MAKE("arg0",
@@ -592,7 +648,8 @@ TEST(option_optional_arg) {
{ "output", "foo.txt" },
{}
},
NULL);
NULL,
false);
/* Long option without = does NOT consume the next arg */
test_option_parse_one(STRV_MAKE("arg0",
@@ -602,7 +659,8 @@ TEST(option_optional_arg) {
{ "output", NULL },
{}
},
STRV_MAKE("foo.txt"));
STRV_MAKE("foo.txt"),
false);
/* Short option with inline arg */
test_option_parse_one(STRV_MAKE("arg0",
@@ -612,7 +670,8 @@ TEST(option_optional_arg) {
{ "output", "foo.txt" },
{}
},
NULL);
NULL,
false);
/* Short option without inline arg does NOT consume the next arg */
test_option_parse_one(STRV_MAKE("arg0",
@@ -622,7 +681,8 @@ TEST(option_optional_arg) {
{ "output", NULL },
{}
},
STRV_MAKE("foo.txt"));
STRV_MAKE("foo.txt"),
false);
/* Optional arg option at end of argv */
test_option_parse_one(STRV_MAKE("arg0",
@@ -632,7 +692,8 @@ TEST(option_optional_arg) {
{ "output", NULL },
{}
},
NULL);
NULL,
false);
/* Mixed: optional arg with other options */
test_option_parse_one(STRV_MAKE("arg0",
@@ -646,7 +707,8 @@ TEST(option_optional_arg) {
{ "help" },
{}
},
NULL);
NULL,
false);
/* Short combo: -ho (h then o with no arg) */
test_option_parse_one(STRV_MAKE("arg0",
@@ -657,7 +719,8 @@ TEST(option_optional_arg) {
{ "output", NULL },
{}
},
STRV_MAKE("pos1"));
STRV_MAKE("pos1"),
false);
/* Short combo: -hobar (h then o with inline arg "bar") */
test_option_parse_one(STRV_MAKE("arg0",
@@ -668,7 +731,8 @@ TEST(option_optional_arg) {
{ "output", "bar" },
{}
},
NULL);
NULL,
false);
}
/* Test the OPTION, OPTION_LONG, OPTION_SHORT, OPTION_FULL, OPTION_GROUP macros
@@ -1027,4 +1091,145 @@ TEST(option_macros) {
"-h"));
}
/* Test the pattern used by nspawn's --user: an optional-arg option that also
* 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" },
{}
};
/* --user=NAME: optional arg provided via = */
test_option_parse_one(STRV_MAKE("arg0",
"--user=root"),
options,
(Entry[]) {
{ "user", "root" },
{}
},
NULL,
/* stop_at_first_nonoption= */ false);
/* --user without arg: next arg is an option, so no consumption */
test_option_parse_one(STRV_MAKE("arg0",
"--user",
"--help"),
options,
(Entry[]) {
{ "user", NULL },
{ "help" },
{}
},
NULL,
/* stop_at_first_nonoption= */ false);
/* --user without arg: next arg is positional (doesn't start with -).
* The option parser returns NULL for the arg. The caller would then
* use option_parser_next_arg/consume_next_arg to grab it. */
{
char **argv = STRV_MAKE("arg0", "--user", "someuser", "pos1");
int argc = strv_length(argv);
OptionParser state = { argc, argv };
const Option *opt;
const char *arg;
ASSERT_OK_POSITIVE(option_parse(options, options + 3, &state, &opt, &arg));
ASSERT_STREQ(opt->long_code, "user");
ASSERT_NULL(arg);
ASSERT_STREQ(option_parser_next_arg(&state), "someuser");
ASSERT_STREQ(option_parser_consume_next_arg(&state), "someuser");
ASSERT_EQ(option_parse(options, options + 3, &state, &opt, &arg), 0);
ASSERT_TRUE(strv_equal(option_parser_get_args(&state), STRV_MAKE("pos1")));
}
/* --user at end of args: no next arg, so scope mode */
{
char **argv = STRV_MAKE("arg0", "--user");
int argc = strv_length(argv);
OptionParser state = { argc, argv };
const Option *opt;
const char *arg;
ASSERT_OK_POSITIVE(option_parse(options, options + 3, &state, &opt, &arg));
ASSERT_STREQ(opt->long_code, "user");
ASSERT_NULL(arg);
ASSERT_NULL(option_parser_next_arg(&state));
ASSERT_NULL(option_parser_consume_next_arg(&state));
ASSERT_EQ(option_parse(options, options + 3, &state, &opt, &arg), 0);
ASSERT_TRUE(strv_isempty(option_parser_get_args(&state)));
}
/* --user followed by -u (option): scope mode, -u gets its own processing */
{
char **argv = STRV_MAKE("arg0", "--user", "-u", "nobody");
int argc = strv_length(argv);
OptionParser state = { argc, argv };
const Option *opt;
const char *arg;
ASSERT_OK_POSITIVE(option_parse(options, options + 3, &state, &opt, &arg));
ASSERT_STREQ(opt->long_code, "user");
ASSERT_NULL(arg);
ASSERT_STREQ(option_parser_next_arg(&state), "-u");
ASSERT_OK_POSITIVE(option_parse(options, options + 3, &state, &opt, &arg));
ASSERT_STREQ(opt->long_code, "uid");
ASSERT_STREQ(arg, "nobody");
ASSERT_NULL(option_parser_next_arg(&state));
ASSERT_NULL(option_parser_consume_next_arg(&state));
ASSERT_EQ(option_parse(options, options + 3, &state, &opt, &arg), 0);
ASSERT_TRUE(strv_isempty(option_parser_get_args(&state)));
}
/* "Functional test": --user followed by -u (option): scope mode, -u gets its own processing,
* handled like in a real option parser. */
{
char **argv = STRV_MAKE("arg0", "--user", "-u", "nobody", "nogroup", "--user=nobody", "--user");
int argc = strv_length(argv);
OptionParser state = { argc, argv };
const Option *opt;
const char *arg;
int scope_seen = 0;
int nobody_seen = 0;
for (int c; (c = option_parse(options, options + 3, &state, &opt, &arg)) != 0; ) {
ASSERT_OK(c);
if (streq_ptr(opt->long_code, "user")) {
if (!arg) {
const char *t = option_parser_next_arg(&state);
if (t && t[0] != '-')
arg = option_parser_consume_next_arg(&state);
}
if (arg) {
ASSERT_STREQ(arg, "nobody");
nobody_seen ++;
} else
scope_seen ++;
} else if (streq_ptr(opt->long_code, "uid")) {
ASSERT_STREQ(arg, "nobody");
nobody_seen ++;
}
}
ASSERT_EQ(nobody_seen, 2);
ASSERT_EQ(scope_seen, 2);
ASSERT_TRUE(strv_equal(option_parser_get_args(&state), STRV_MAKE("nogroup")));
}
}
DEFINE_TEST_MAIN(LOG_DEBUG);
src/vmspawn/vmspawn.c
+453 -598
View File
File diff suppressed because it is too large Load Diff