Hi all, On Thu, Sep 10, 2026 at 11:46 AM Nathan Bossart <[email protected]> wrote: > > 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. >
I tested both v2 patches on the latest master and was able to reproduce both reported issues with the in-tree getopt_long() implementation before applying the patches with the following observations: --stats --limit 5 incorrectly skipped --limit and left 5 as a positional argument. mydb --jobs incorrectly consumed mydb as the argument to --jobs. After applying both patches, both cases behaved correctly. And I ran the affected TAP tests and everything got passed. Also I reviewed the changes in src/port/getopt_long.c and the corresponding test additions. I did not find any issues with the v2 patches. The patch looks good to me. Regards, Solai
