From: Zbigniew Jędrzejewski-Szmek Date: Tue, 28 Apr 2026 06:45:03 +0000 (+0200) Subject: shared/options: add an assert loop to verify ordering X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4c640beba6faf717d7f145036af4fa5ba430e6a9;p=thirdparty%2Fsystemd.git 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. --- diff --git a/src/shared/options.c b/src/shared/options.c index 364b57792c8..227ba8ffa35 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 8b8fc9ed4a8..7bb73dd5811 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 a77964df4e1..150c2b2a566 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" }, {} };