Teach in-tree getopt_long() to move non-options to the end of argv.

Unlike the other implementations of getopt_long() I could find, the
in-tree implementation does not reorder non-options to the end of
argv.  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.
This quirk is periodically missed by hackers (e.g., 869aa40a27,
ffd398021c, and d9ddc50baf).  This commit introduces the
aforementioned non-option reordering behavior to the in-tree
getopt_long() implementation.

Special thanks to Noah Misch for his help verifying behavior on
AIX.

Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20230609232257.GA121461%40nathanxps13
This commit is contained in:
Nathan Bossart 2023-07-12 20:34:39 -07:00
parent b6e1157e7d
commit 411b720343
2 changed files with 44 additions and 19 deletions

View File

@ -42,9 +42,8 @@ $node->issues_sql_like(
'add a role as a member with admin option of the newly created role');
$node->issues_sql_like(
[
'createuser', '-m',
'regress_user3', '-m',
'regress user #4', 'REGRESS_USER5'
'createuser', 'REGRESS_USER5', '-m', 'regress_user3',
'-m', 'regress user #4'
],
qr/statement: CREATE ROLE "REGRESS_USER5" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS ROLE regress_user3,"regress user #4";/,
'add a role as a member of the newly created role');
@ -73,11 +72,14 @@ $node->issues_sql_like(
qr/statement: CREATE ROLE regress_user11 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
'--role');
$node->issues_sql_like(
[ 'createuser', '--member-of', 'regress_user1', 'regress_user12' ],
[ 'createuser', 'regress_user12', '--member-of', 'regress_user1' ],
qr/statement: CREATE ROLE regress_user12 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
'--member-of');
$node->command_fails([ 'createuser', 'regress_user1' ],
'fails if role already exists');
$node->command_fails(
[ 'createuser', 'regress_user1', '-m', 'regress_user2', 'regress_user3' ],
'fails for too many non-options');
done_testing();

View File

@ -50,8 +50,11 @@
* This implementation does not use optreset. Instead, we guarantee that
* it can be restarted on a new argv array after a previous call returned -1,
* if the caller resets optind to 1 before the first call of the new series.
* (Internally, this means we must be sure to reset "place" to EMSG before
* returning -1.)
* (Internally, this means we must be sure to reset "place" to EMSG,
* "nonopt_start" to -1, and "force_nonopt" to false before returning -1.)
*
* Note that this routine reorders the pointers in argv (despite the const
* qualifier) so that all non-options will be at the end when -1 is returned.
*/
int
getopt_long(int argc, char *const argv[],
@ -60,38 +63,58 @@ getopt_long(int argc, char *const argv[],
{
static char *place = EMSG; /* option letter processing */
char *oli; /* option letter list index */
static int nonopt_start = -1;
static bool force_nonopt = false;
if (!*place)
{ /* update scanning pointer */
if (optind >= argc)
char **args = (char **) argv;
retry:
/*
* If we are out of arguments or only non-options remain, return -1.
*/
if (optind >= argc || optind == nonopt_start)
{
place = EMSG;
nonopt_start = -1;
force_nonopt = false;
return -1;
}
place = argv[optind];
if (place[0] != '-')
/*
* An argument is a non-option if it meets any of the following
* criteria: it follows an argument that is equivalent to the string
* "--", it does not start with '-', or it is equivalent to the string
* "-". When we encounter a non-option, we move it to the end of argv
* (after shifting all remaining arguments over to make room), and
* then we try again with the next argument.
*/
if (force_nonopt || place[0] != '-' || place[1] == '\0')
{
place = EMSG;
return -1;
for (int i = optind; i < argc - 1; i++)
args[i] = args[i + 1];
args[argc - 1] = place;
if (nonopt_start == -1)
nonopt_start = argc - 1;
else
nonopt_start--;
goto retry;
}
place++;
if (!*place)
{
/* treat "-" as not being an option */
place = EMSG;
return -1;
}
if (place[0] == '-' && place[1] == '\0')
{
/* found "--", treat it as end of options */
++optind;
place = EMSG;
return -1;
force_nonopt = true;
goto retry;
}
if (place[0] == '-' && place[1])