On Sat, Aug 29, 2026 at 12:57:00PM -0400, Sehrope Sarkuni wrote: > While reworking the pg_waldump TAP test [1] I ran "pg_waldump --stats > --limit 5" and it failed on Windows CI with 'could not locate WAL file > "5"'. For a long option with optional_argument and no "=", the port > getopt_long() increments optind in the missing-argument branch and > again at the end of the long option path, so the next option is > skipped and its value becomes a positional argument. The same branch > returns BADARG when optstring starts with a colon although nothing is > missing. > > [...] > > 0003 fixes a second problem the tests turned up. The port moves each > non-option to the end of argv as soon as it sees it, so by the time a > following option looks for its argument, a non-option that came before > it on the command line is already sitting after it and gets taken: > "pg_amcheck mydb --jobs" parses as --jobs=mydb rather than reporting > the missing argument. The system getopt_long() reports the missing > argument. The fix bounds the argument search by the start of the > moved block. 0004 adds the test cases for it, kept separate in case > 0003 is backpatched without the test module.
Nice finds. I've attached what I have staged for commit. I've tried to simplify the diffs a bit, and I also got rid of the new test module in favor of adding cases to existing test files. I don't think we need to bother back-patching 0001; the bug went undiscovered for ~23 years. I can try back-patching 0002 to v17, though. -- nathan
>From f04a179b4f24f79f54ce81c6060f45c86a803c34 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Fri, 4 Sep 2026 16:37:01 -0500 Subject: [PATCH v2 1/2] Fix optional-argument handling in in-tree getopt_long(). A long option with an optional argument that is given without "=" advances optind twice, so the following argument is skipped. For example, "pg_waldump --stats --limit 5" complains that it cannot locate WAL file "5". The same path also returns BADARG when optstring starts with a colon, even though nothing is missing. To fix, handle optional arguments before the missing-argument code, which then only needs to deal with required arguments. This is a bug fix and could be back-patched, but since this issue went unnoticed for 23 years, I'm not going to bother. Author: Sehrope Sarkuni <[email protected]> Discussion: https://postgr.es/m/CAH7T-arxDuVCSkorO%3Dk7%2BM-_JV0JFzMpN_EtKMyD2K0RDqZ2OA%40mail.gmail.com --- src/bin/pg_waldump/t/001_basic.pl | 3 +-- src/port/getopt_long.c | 11 +++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl index 8beac19eaff..7b33efc6299 100644 --- a/src/bin/pg_waldump/t/001_basic.pl +++ b/src/bin/pg_waldump/t/001_basic.pl @@ -340,11 +340,10 @@ sub test_pg_waldump my ($stdout, $stderr); my $result = IPC::Run::run [ - 'pg_waldump', + 'pg_waldump', @opts, '--start' => $startlsn, '--end' => $endlsn, '--path' => $path, - @opts ], '>' => \$stdout, '2>' => \$stderr; diff --git a/src/port/getopt_long.c b/src/port/getopt_long.c index 2e869fed58b..0a9a50189f1 100644 --- a/src/port/getopt_long.c +++ b/src/port/getopt_long.c @@ -137,8 +137,9 @@ retry: { if (place[namelen] == '=') optarg = place + namelen + 1; - else if (optind < argc - 1 && - has_arg == required_argument) + else if (has_arg == optional_argument) + optarg = NULL; + else if (optind < argc - 1) { optind++; optarg = argv[optind]; @@ -152,16 +153,14 @@ retry: return BADARG; } - if (opterr && has_arg == required_argument) + if (opterr) fprintf(stderr, "%s: option requires an argument -- %s\n", argv[0], place); place = EMSG; - if (has_arg == required_argument) - return BADCH; - optarg = NULL; + return BADCH; } } else -- 2.55.0
>From b0fcd1a01b01e9e15d76dcadfae13f33f9ab8bb1 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Fri, 4 Sep 2026 16:47:53 -0500 Subject: [PATCH v2 2/2] Fix option argument lookup in in-tree getopt_long(). The in-tree getopt_long() moves each non-option to the end of argv as soon as it finds one, which puts a non-option that preceded an option right where the option's argument lookup expects to find it. For example, "vacuumdb postgres --jobs" takes "postgres" as the number of jobs instead of complaining that --jobs is missing its argument. To fix, stop the argument lookups at the start of the moved non-options, which we already track to know when to stop scanning. Oversight in commit 411b720343. Author: Sehrope Sarkuni <[email protected]> Discussion: https://postgr.es/m/CAH7T-arxDuVCSkorO%3Dk7%2BM-_JV0JFzMpN_EtKMyD2K0RDqZ2OA%40mail.gmail.com Backpatch-through: 17 --- src/bin/scripts/t/100_vacuumdb.pl | 4 ++++ src/port/getopt_long.c | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl index 7c4e35a6717..b78fa2a38de 100644 --- a/src/bin/scripts/t/100_vacuumdb.pl +++ b/src/bin/scripts/t/100_vacuumdb.pl @@ -240,6 +240,10 @@ $node->command_fails_like( [ 'vacuumdb', '--all', 'postgres' ], qr/cannot vacuum all databases and a specific one at the same time/, 'cannot use option --all and a dbname as argument at the same time'); +$node->command_fails_like( + [ 'vacuumdb', 'postgres', '--jobs' ], + qr/requires an argument/, + 'option missing its argument after a non-option'); $node->safe_psql( 'postgres', q| diff --git a/src/port/getopt_long.c b/src/port/getopt_long.c index 0a9a50189f1..f2edadb59d5 100644 --- a/src/port/getopt_long.c +++ b/src/port/getopt_long.c @@ -66,6 +66,9 @@ getopt_long(int argc, char *const argv[], static int nonopt_start = -1; static bool force_nonopt = false; + if (nonopt_start == -1) + nonopt_start = argc; + if (!*place) { /* update scanning pointer */ char **args = (char **) argv; @@ -75,7 +78,7 @@ retry: /* * If we are out of arguments or only non-options remain, return -1. */ - if (optind >= argc || optind == nonopt_start) + if (optind >= nonopt_start) { place = EMSG; nonopt_start = -1; @@ -99,10 +102,7 @@ retry: args[i] = args[i + 1]; args[argc - 1] = place; - if (nonopt_start == -1) - nonopt_start = argc - 1; - else - nonopt_start--; + nonopt_start--; goto retry; } @@ -139,7 +139,7 @@ retry: optarg = place + namelen + 1; else if (has_arg == optional_argument) optarg = NULL; - else if (optind < argc - 1) + else if (optind < nonopt_start - 1) { optind++; optarg = argv[optind]; @@ -222,7 +222,7 @@ retry: { /* need an argument */ if (*place) /* no white space */ optarg = place; - else if (argc <= ++optind) + else if (nonopt_start <= ++optind) { /* no arg */ place = EMSG; if (*optstring == ':') -- 2.55.0
