public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: add non-option reordering to in-tree getopt_long
Date: Thu, 15 Jun 2023 21:58:28 -0700
Message-ID: <20230616045828.GA972663@nathanxps13> (raw)
In-Reply-To: <[email protected]>
References: <20230614212816.GA492731@nathanxps13>
	<[email protected]>
	<20230614224608.GA625054@nathanxps13>
	<[email protected]>
	<20230616000959.GA922959@nathanxps13>
	<[email protected]>

On Fri, Jun 16, 2023 at 10:30:09AM +0900, Michael Paquier wrote:
> On Thu, Jun 15, 2023 at 05:09:59PM -0700, Nathan Bossart wrote:
>> I've attached a new version of the patch that omits the
>> POSIXLY_CORRECT stuff.
> 
> This looks OK at quick glance, though you may want to document at the
> top of getopt_long() the reordering business with the non-options?

I added a comment to this effect in v3.  I also noticed that '-' wasn't
properly handled as a non-option, so I've tried to fix that as well.

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


Attachments:

  [text/x-diff] v3-0001-Teach-in-tree-getopt_long-to-move-non-options-to-.patch (4.0K, ../20230616045828.GA972663@nathanxps13/2-v3-0001-Teach-in-tree-getopt_long-to-move-non-options-to-.patch)
  download | inline diff:
From e31fa0ab5237d3ad35bdb44404fd5b5eeea3f5c6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Fri, 9 Jun 2023 15:51:58 -0700
Subject: [PATCH v3 1/1] Teach in-tree getopt_long() to move non-options to the
 end of argv.

---
 src/bin/scripts/t/040_createuser.pl | 10 ++++---
 src/port/getopt_long.c              | 45 +++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/src/bin/scripts/t/040_createuser.pl b/src/bin/scripts/t/040_createuser.pl
index 9ca282181d..ba40ab11a3 100644
--- a/src/bin/scripts/t/040_createuser.pl
+++ b/src/bin/scripts/t/040_createuser.pl
@@ -42,9 +42,9 @@ $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 +73,13 @@ $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..da233728e1 100644
--- a/src/port/getopt_long.c
+++ b/src/port/getopt_long.c
@@ -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 and
+ * nonopt_start to -1 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,37 +63,55 @@ 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;
 
 	if (!*place)
 	{							/* update scanning pointer */
 		if (optind >= argc)
 		{
 			place = EMSG;
+			nonopt_start = -1;
 			return -1;
 		}
 
+retry:
 		place = argv[optind];
 
-		if (place[0] != '-')
+		if (place[0] != '-' || place[1] == '\0')
 		{
-			place = EMSG;
-			return -1;
-		}
+			char	  **args = (char **) argv;
 
-		place++;
+			/*
+			 * If only non-options remain, return -1.  Else, move the
+			 * non-option to the end and try again.
+			 */
+			if (optind == nonopt_start)
+			{
+				place = EMSG;
+				nonopt_start = -1;
+				return -1;
+			}
 
-		if (!*place)
-		{
-			/* treat "-" as not being an option */
-			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[0] == '-' && place[1] == '\0')
 		{
 			/* found "--", treat it as end of options */
 			++optind;
 			place = EMSG;
+			nonopt_start = -1;
 			return -1;
 		}
 
-- 
2.25.1



view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: add non-option reordering to in-tree getopt_long
  In-Reply-To: <20230616045828.GA972663@nathanxps13>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox