Skip to content

Commit 411b720

Browse files
nathan-bossartnathan-bossart
committed
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., 869aa40, ffd3980, and d9ddc50). 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
1 parent b6e1157 commit 411b720

File tree

2 files changed

+44
-19
lines changed

2 files changed

+44
-19
lines changed

‎src/bin/scripts/t/040_createuser.pl

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,8 @@
4242
'add a role as a member with admin option of the newly created role');
4343
$node->issues_sql_like(
4444
[
45-
'createuser', '-m',
46-
'regress_user3', '-m',
47-
'regress user #4', 'REGRESS_USER5'
45+
'createuser', 'REGRESS_USER5', '-m', 'regress_user3',
46+
'-m', 'regress user #4'
4847
],
4948
qr/statement: CREATE ROLE "REGRESS_USER5" NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS ROLE regress_user3,"regress user #4";/,
5049
'add a role as a member of the newly created role');
@@ -73,11 +72,14 @@
7372
qr/statement: CREATE ROLE regress_user11 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
7473
'--role');
7574
$node->issues_sql_like(
76-
[ 'createuser', '--member-of', 'regress_user1', 'regress_user12' ],
75+
[ 'createuser', 'regress_user12', '--member-of', 'regress_user1' ],
7776
qr/statement: CREATE ROLE regress_user12 NOSUPERUSER NOCREATEDB NOCREATEROLE INHERIT LOGIN NOREPLICATION NOBYPASSRLS IN ROLE regress_user1;/,
7877
'--member-of');
7978

8079
$node->command_fails([ 'createuser', 'regress_user1' ],
8180
'fails if role already exists');
81+
$node->command_fails(
82+
[ 'createuser', 'regress_user1', '-m', 'regress_user2', 'regress_user3' ],
83+
'fails for too many non-options');
8284

8385
done_testing();

‎src/port/getopt_long.c

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@
5050
* This implementation does not use optreset. Instead, we guarantee that
5151
* it can be restarted on a new argv array after a previous call returned -1,
5252
* if the caller resets optind to 1 before the first call of the new series.
53-
* (Internally, this means we must be sure to reset "place" to EMSG before
54-
* returning -1.)
53+
* (Internally, this means we must be sure to reset "place" to EMSG,
54+
* "nonopt_start" to -1, and "force_nonopt" to false before returning -1.)
55+
*
56+
* Note that this routine reorders the pointers in argv (despite the const
57+
* qualifier) so that all non-options will be at the end when -1 is returned.
5558
*/
5659
int
5760
getopt_long(int argc, char *const argv[],
@@ -60,38 +63,58 @@ getopt_long(int argc, char *const argv[],
6063
{
6164
static char *place = EMSG; /* option letter processing */
6265
char *oli; /* option letter list index */
66+
static int nonopt_start = -1;
67+
static bool force_nonopt = false;
6368

6469
if (!*place)
6570
{ /* update scanning pointer */
66-
if (optind >= argc)
71+
char **args = (char **) argv;
72+
73+
retry:
74+
75+
/*
76+
* If we are out of arguments or only non-options remain, return -1.
77+
*/
78+
if (optind >= argc || optind == nonopt_start)
6779
{
6880
place = EMSG;
81+
nonopt_start = -1;
82+
force_nonopt = false;
6983
return -1;
7084
}
7185

7286
place = argv[optind];
7387

74-
if (place[0] != '-')
88+
/*
89+
* An argument is a non-option if it meets any of the following
90+
* criteria: it follows an argument that is equivalent to the string
91+
* "--", it does not start with '-', or it is equivalent to the string
92+
* "-". When we encounter a non-option, we move it to the end of argv
93+
* (after shifting all remaining arguments over to make room), and
94+
* then we try again with the next argument.
95+
*/
96+
if (force_nonopt || place[0] != '-' || place[1] == '\0')
7597
{
76-
place = EMSG;
77-
return -1;
78-
}
98+
for (int i = optind; i < argc - 1; i++)
99+
args[i] = args[i + 1];
100+
args[argc - 1] = place;
79101

80-
place++;
102+
if (nonopt_start == -1)
103+
nonopt_start = argc - 1;
104+
else
105+
nonopt_start--;
81106

82-
if (!*place)
83-
{
84-
/* treat "-" as not being an option */
85-
place = EMSG;
86-
return -1;
107+
goto retry;
87108
}
88109

110+
place++;
111+
89112
if (place[0] == '-' && place[1] == '\0')
90113
{
91114
/* found "--", treat it as end of options */
92115
++optind;
93-
place = EMSG;
94-
return -1;
116+
force_nonopt = true;
117+
goto retry;
95118
}
96119

97120
if (place[0] == '-' && place[1])

0 commit comments

Comments
 (0)