add non-option reordering to in-tree getopt_long - Mailing list pgsql-hackers

From Nathan Bossart
Subject add non-option reordering to in-tree getopt_long
Date
Msg-id 20230609232257.GA121461@nathanxps13
Whole thread Raw
Responses Re: add non-option reordering to in-tree getopt_long
List pgsql-hackers
While working on 2dcd157, I noticed cfbot failures for Windows due to tests
with commands that had non-options specified before options.  For example,
"createuser myrole -a myadmin" specified a non-option (myrole) before the
option/argument pair (-a admin).  To get the tests passing for Windows,
non-options must be placed at the end (e.g., "createuser -a myadmin
myrole").  This same problem was encountered while working on 08951a7 [0],
but it was again fortunately caught with cfbot.  Others have not been so
lucky [1] [2] [3].

The reason for this discrepancy is because Windows uses the in-tree
implementation of getopt_long(), which differs from the other
implementations I've found in that it doesn't reorder non-options to the
end of argv by default.  Instead, it returns -1 as soon as the first
non-option is found, even if there are other options listed afterwards.  By
moving non-options to the end of argv, getopt_long() can parse all
specified options and return -1 when only non-options remain.  The
implementations I reviewed all reorder argv unless the POSIXLY_CORRECT
environment variable is set or the "optstring" argument begins with '+'.

The best reasons I can think of to keep the current behavior are 1)
reordering involves writing to the original argv array, which could be
risky (as noted by Tom [4]) and 2) any systems with a getopt_long()
implementation that doesn't reorder non-options could begin failing tests
that take advantage of this behavior.  However, this quirk in the in-tree
getopt_long() is periodically missed by hackers, the only systems I'm aware
of that use it are Windows and AIX, and every other implementation of
getopt_long() I saw reorders non-options by default.  Furthermore, C99
omits const decorations in main()'s signature, so modifying argv might not
be too scary.

Thus, I propose introducing this non-option reordering behavior but
allowing it to be disabled via the POSIXLY_CORRECT environment variable.
The attached patch is my first attempt at implementing this proposal.  I
don't think we need to bother with handling '+' at the beginning of
"optstring" since it seems unlikely to be used in PostgreSQL, but it would
probably be easy enough to add if folks want it.

I briefly looked at getopt() and concluded that we should probably retain
its POSIX-compliant behavior for non-options, as reordering support seems
much less universal than with getopt_long().  AFAICT all client utilities
use getopt_long(), anyway.

Thoughts?

[0] https://postgr.es/m/20220525.110752.305692011781436338.horikyota.ntt%40gmail.com
[1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=869aa40
[2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=ffd3980
[3] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=d9ddc50
[4] https://postgr.es/m/20539.1237314382%40sss.pgh.pa.us

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Attachment

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: Let's make PostgreSQL multi-threaded
Next
From: Gurjeet Singh
Date:
Subject: Re: Fix search_path for all maintenance commands