From 7c978c11c8012cbfa67646bfc37849cb061f4e07 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 7 Jul 2023 20:00:47 -0700
Subject: [PATCH v7 1/1] 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.  The only
systems I'm aware of that use it are Windows and AIX, both of which
have been tested.

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

Reviewed-by: Kyotaro Horiguchi, Michael Paquier
Discussion: https://postgr.es/m/20230609232257.GA121461%40nathanxps13
---
 src/bin/scripts/t/040_createuser.pl | 10 +++---
 src/port/getopt_long.c              | 54 ++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index 9ca282181d..3290e30c88 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -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();
diff --git a/src/port/getopt_long.c b/src/port/getopt_long.c
index c989276988..b290a2bab9 100644
--- a/src/port/getopt_long.c
+++ b/src/port/getopt_long.c
@@ -50,8 +50,9 @@
  * 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.)
+ *
+ * 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,40 +61,59 @@ 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] != '-' || strcmp("-", place) == 0)
 		{
-			place = EMSG;
-			return -1;
-		}
+			for (int i = optind; i < argc - 1; i++)
+				args[i] = args[i + 1];
+			args[argc - 1] = place;
 
-		place++;
+			if (nonopt_start == -1)
+				nonopt_start = argc - 1;
+			else
+				nonopt_start--;
 
-		if (!*place)
-		{
-			/* treat "-" as not being an option */
-			place = EMSG;
-			return -1;
+			goto retry;
 		}
-
-		if (place[0] == '-' && place[1] == '\0')
+		else if (strcmp("--", place) == 0)
 		{
 			/* found "--", treat it as end of options */
 			++optind;
-			place = EMSG;
-			return -1;
+			force_nonopt = true;
+			goto retry;
 		}
 
+		place++;
+
 		if (place[0] == '-' && place[1])
 		{
 			/* long option */
-- 
2.25.1

