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.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2026-03-26 10:31:12 +01:00
parent 9e8658b887
commit 5dd2ae14db
5 changed files with 22 additions and 19 deletions
+1 -1
View File
@@ -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;
+18 -12
View File
@@ -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[],
+1 -4
View File
@@ -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[],
+1 -1
View File
@@ -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(
+1 -1
View File
@@ -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;