On Tue, Jul 11, 2023 at 04:16:09PM +0900, Kyotaro Horiguchi wrote: > I like it. We don't need to overcomplicate things just for the sake of > speed here. Plus, this version looks the most simple to me. That being > said, it might be better if the last term is positioned in the second > place. This is mainly because a lone hyphen is less common than words > that starts with characters other than a pyphen.
Sure. І did it this way in v7. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
>From 7c978c11c8012cbfa67646bfc37849cb061f4e07 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <nat...@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
