shared/options: stop removing items from argv array

If we remove "--" from argv, the argc parameter stops being valid. But
that state is effectively global, albeit readonly, and somebody looking
at argv+argc after that will see an inconsistent state. Let's behave
like the libc parsing code and instead just shuffle things around in
argv, so that the argv+argc pair remains consistent.

This allows the code to calculate how many options are remaining to
be simplified.

diff --git a/src/ac-power/ac-power.c b/src/ac-power/ac-power.c
index ec07a914c59a25f429de683e7d4e2b910949358d..e6b58810a6d004fa3f8c1ad2a12a4e35b0a324ac 100644 (file)
--- a/src/ac-power/ac-power.c
+++ b/src/ac-power/ac-power.c
@@ -68,7 +68,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
                 }
 
-        if (option_parser_get_n_args(&state, argc, argv) > 0)
+        if (option_parser_get_n_args(&state, argc) > 0)
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program takes no arguments.");
 
         return 1;

diff --git a/src/shared/options.c b/src/shared/options.c
index 8ea22c9b7a18d63e862800062f385fe9c53552d0..508db28ea0a908a58d2a286b6aafdd055ebeda2d 100644 (file)
--- a/src/shared/options.c
+++ b/src/shared/options.c
@@ -25,14 +25,6 @@ static bool option_is_metadata(const Option *opt) {
                 FLAGS_SET(ASSERT_PTR(opt)->flags, OPTION_HELP_ENTRY);
 }
 
-static void kill_arg(char* argv[], int argc, int index) {
-        assert(index < argc);
-        assert(!argv[argc]);
-
-        /* Eliminate argv[index] */
-        memmove(argv + index, argv + index + 1, (argc - index) * sizeof(char*));
-}
-
 static void shift_arg(char* argv[], int target, int source) {
         assert(argv);
         assert(target <= source);
@@ -108,9 +100,10 @@ int option_parse(
                                 return 0;
 
                         if (streq(argv[state->optind], "--")) {
-                                /* No more options. Eliminate "--" so that the list of positional args is clean. */
-                                kill_arg(argv, argc, state->optind);
-                                return 0;
+                                /* No more options. Move "--" before positional args so that
+                                 * the list of positional args is clean. */
+                                shift_arg(argv, state->positional_offset++, state->optind++);
+                                state->parsing_stopped = true;
                         }
 
                         if (state->parsing_stopped)
@@ -243,12 +236,25 @@ int option_parse(
 
 char** option_parser_get_args(const OptionParser *state, int argc, char *argv[]) {
         /* Returns positional args as a strv.
-         * If "--" was found, it has been removed. */
+         * 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);
+
         return argv + state->positional_offset;
 }
 
+size_t option_parser_get_n_args(const OptionParser *state, int argc) {
+        assert(state->optind > 0);
+        assert(state->optind == argc || state->parsing_stopped);
+        assert(state->positional_offset <= argc);
+
+        return argc - state->positional_offset;
+}
+
 int _option_parser_get_help_table(
                 const Option options[],
                 const Option options_end[],

diff --git a/src/shared/options.h b/src/shared/options.h
index fd2d048db00c495776620c80d4cf167abf1c27da..4895c10cbd0140097e88299cabbcbaf81d5e4a25 100644 (file)
--- a/src/shared/options.h
+++ b/src/shared/options.h
@@ -2,7 +2,6 @@
 #pragma once
 
 #include "shared-forward.h"
-#include "strv.h"
 
 typedef enum OptionFlags {
         OPTION_OPTIONAL_ARG  = 1U << 0,  /* Same as optional_argument in getopt */
@@ -98,9 +97,7 @@ int option_parse(
         FOREACH_OPTION_FULL(parser, opt, argc, argv, /* ret_o= */ NULL, ret_a, on_error)
 
 char** option_parser_get_args(const OptionParser *state, int argc, char *argv[]);
-static inline size_t option_parser_get_n_args(const OptionParser *state, int argc, char *argv[]) {
-        return strv_length(option_parser_get_args(state, argc, argv));
-}
+size_t option_parser_get_n_args(const OptionParser *state, int argc);
 
 int _option_parser_get_help_table(
                 const Option options[],

diff --git a/src/test/test-options.c b/src/test/test-options.c
index 201d16d51f8951e3b5b75f0d41388cb52c302b55..e849365a2f2ee8ae270658565d5aac30f3379f92 100644 (file)
--- a/src/test/test-options.c
+++ b/src/test/test-options.c
@@ -58,7 +58,7 @@ static void test_option_parse_one(
         ASSERT_TRUE(strv_equal(args, remaining));
         ASSERT_STREQ(argv[0], saved_argv0);
 
-        ASSERT_EQ(option_parser_get_n_args(&state, argc, argv), strv_length(remaining));
+        ASSERT_EQ(option_parser_get_n_args(&state, argc), strv_length(remaining));
 }
 
 static void test_option_invalid_one(

diff --git a/src/update-done/update-done.c b/src/update-done/update-done.c
index 03e5479aaefa4eeb9b437b041e49e76b4b784680..7fb1c58d19bac5ae59a8a615c90d6a9640cd720d 100644 (file)
--- a/src/update-done/update-done.c
+++ b/src/update-done/update-done.c
@@ -112,7 +112,7 @@ static int parse_argv(int argc, char *argv[]) {
                         break;
                 }
 
-        if (option_parser_get_n_args(&state, argc, argv) > 0)
+        if (option_parser_get_n_args(&state, argc) > 0)
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program takes no arguments.");
 
         return 1;