Simplify option handling in pg_ctl.
authorNathan Bossart <nathan@postgresql.org>
Fri, 14 Jul 2023 19:35:54 +0000 (12:35 -0700)
committerNathan Bossart <nathan@postgresql.org>
Fri, 14 Jul 2023 19:35:54 +0000 (12:35 -0700)
Now that the in-tree getopt_long() moves non-options to the end of
argv (see commit 411b720343), we can remove pg_ctl's workaround for
getopt_long() implementations that don't reorder argv.

Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/20230713034903.GA991765%40nathanxps13

src/bin/pg_ctl/pg_ctl.c

index 465948e70749cf970d755554213c21144611dafe..807d7023a997fda437ed193d49510c0d88abfae6 100644 (file)
@@ -2260,163 +2260,151 @@ main(int argc, char **argv)
    if (env_wait != NULL)
        wait_seconds = atoi(env_wait);
 
-   /*
-    * 'Action' can be before or after args so loop over both. Some
-    * getopt_long() implementations will reorder argv[] to place all flags
-    * first (GNU?), but we don't rely on it. Our /port version doesn't do
-    * that.
-    */
-   optind = 1;
-
    /* process command-line options */
-   while (optind < argc)
+   while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
+                           long_options, &option_index)) != -1)
    {
-       while ((c = getopt_long(argc, argv, "cD:e:l:m:N:o:p:P:sS:t:U:wW",
-                               long_options, &option_index)) != -1)
+       switch (c)
        {
-           switch (c)
-           {
-               case 'D':
-                   {
-                       char       *pgdata_D;
-
-                       pgdata_D = pg_strdup(optarg);
-                       canonicalize_path(pgdata_D);
-                       setenv("PGDATA", pgdata_D, 1);
-
-                       /*
-                        * We could pass PGDATA just in an environment
-                        * variable but we do -D too for clearer postmaster
-                        * 'ps' display
-                        */
-                       pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
-                       free(pgdata_D);
-                       break;
-                   }
-               case 'e':
-                   event_source = pg_strdup(optarg);
-                   break;
-               case 'l':
-                   log_file = pg_strdup(optarg);
-                   break;
-               case 'm':
-                   set_mode(optarg);
-                   break;
-               case 'N':
-                   register_servicename = pg_strdup(optarg);
-                   break;
-               case 'o':
-                   /* append option? */
-                   if (!post_opts)
-                       post_opts = pg_strdup(optarg);
-                   else
-                   {
-                       char       *old_post_opts = post_opts;
-
-                       post_opts = psprintf("%s %s", old_post_opts, optarg);
-                       free(old_post_opts);
-                   }
-                   break;
-               case 'p':
-                   exec_path = pg_strdup(optarg);
-                   break;
-               case 'P':
-                   register_password = pg_strdup(optarg);
-                   break;
-               case 's':
-                   silent_mode = true;
+           case 'D':
+               {
+                   char       *pgdata_D;
+
+                   pgdata_D = pg_strdup(optarg);
+                   canonicalize_path(pgdata_D);
+                   setenv("PGDATA", pgdata_D, 1);
+
+                   /*
+                    * We could pass PGDATA just in an environment variable
+                    * but we do -D too for clearer postmaster 'ps' display
+                    */
+                   pgdata_opt = psprintf("-D \"%s\" ", pgdata_D);
+                   free(pgdata_D);
                    break;
-               case 'S':
+               }
+           case 'e':
+               event_source = pg_strdup(optarg);
+               break;
+           case 'l':
+               log_file = pg_strdup(optarg);
+               break;
+           case 'm':
+               set_mode(optarg);
+               break;
+           case 'N':
+               register_servicename = pg_strdup(optarg);
+               break;
+           case 'o':
+               /* append option? */
+               if (!post_opts)
+                   post_opts = pg_strdup(optarg);
+               else
+               {
+                   char       *old_post_opts = post_opts;
+
+                   post_opts = psprintf("%s %s", old_post_opts, optarg);
+                   free(old_post_opts);
+               }
+               break;
+           case 'p':
+               exec_path = pg_strdup(optarg);
+               break;
+           case 'P':
+               register_password = pg_strdup(optarg);
+               break;
+           case 's':
+               silent_mode = true;
+               break;
+           case 'S':
 #ifdef WIN32
-                   set_starttype(optarg);
+               set_starttype(optarg);
 #else
-                   write_stderr(_("%s: -S option not supported on this platform\n"),
-                                progname);
-                   exit(1);
+               write_stderr(_("%s: -S option not supported on this platform\n"),
+                            progname);
+               exit(1);
 #endif
-                   break;
-               case 't':
-                   wait_seconds = atoi(optarg);
-                   wait_seconds_arg = true;
-                   break;
-               case 'U':
-                   if (strchr(optarg, '\\'))
-                       register_username = pg_strdup(optarg);
-                   else
-                       /* Prepend .\ for local accounts */
-                       register_username = psprintf(".\\%s", optarg);
-                   break;
-               case 'w':
-                   do_wait = true;
-                   break;
-               case 'W':
-                   do_wait = false;
-                   break;
-               case 'c':
-                   allow_core_files = true;
-                   break;
-               default:
-                   /* getopt_long already issued a suitable error message */
-                   do_advice();
-                   exit(1);
-           }
+               break;
+           case 't':
+               wait_seconds = atoi(optarg);
+               wait_seconds_arg = true;
+               break;
+           case 'U':
+               if (strchr(optarg, '\\'))
+                   register_username = pg_strdup(optarg);
+               else
+                   /* Prepend .\ for local accounts */
+                   register_username = psprintf(".\\%s", optarg);
+               break;
+           case 'w':
+               do_wait = true;
+               break;
+           case 'W':
+               do_wait = false;
+               break;
+           case 'c':
+               allow_core_files = true;
+               break;
+           default:
+               /* getopt_long already issued a suitable error message */
+               do_advice();
+               exit(1);
        }
+   }
 
-       /* Process an action */
-       if (optind < argc)
+   /* Process an action */
+   if (optind < argc)
+   {
+       if (strcmp(argv[optind], "init") == 0
+           || strcmp(argv[optind], "initdb") == 0)
+           ctl_command = INIT_COMMAND;
+       else if (strcmp(argv[optind], "start") == 0)
+           ctl_command = START_COMMAND;
+       else if (strcmp(argv[optind], "stop") == 0)
+           ctl_command = STOP_COMMAND;
+       else if (strcmp(argv[optind], "restart") == 0)
+           ctl_command = RESTART_COMMAND;
+       else if (strcmp(argv[optind], "reload") == 0)
+           ctl_command = RELOAD_COMMAND;
+       else if (strcmp(argv[optind], "status") == 0)
+           ctl_command = STATUS_COMMAND;
+       else if (strcmp(argv[optind], "promote") == 0)
+           ctl_command = PROMOTE_COMMAND;
+       else if (strcmp(argv[optind], "logrotate") == 0)
+           ctl_command = LOGROTATE_COMMAND;
+       else if (strcmp(argv[optind], "kill") == 0)
        {
-           if (ctl_command != NO_COMMAND)
+           if (argc - optind < 3)
            {
-               write_stderr(_("%s: too many command-line arguments (first is \"%s\")\n"), progname, argv[optind]);
+               write_stderr(_("%s: missing arguments for kill mode\n"), progname);
                do_advice();
                exit(1);
            }
-
-           if (strcmp(argv[optind], "init") == 0
-               || strcmp(argv[optind], "initdb") == 0)
-               ctl_command = INIT_COMMAND;
-           else if (strcmp(argv[optind], "start") == 0)
-               ctl_command = START_COMMAND;
-           else if (strcmp(argv[optind], "stop") == 0)
-               ctl_command = STOP_COMMAND;
-           else if (strcmp(argv[optind], "restart") == 0)
-               ctl_command = RESTART_COMMAND;
-           else if (strcmp(argv[optind], "reload") == 0)
-               ctl_command = RELOAD_COMMAND;
-           else if (strcmp(argv[optind], "status") == 0)
-               ctl_command = STATUS_COMMAND;
-           else if (strcmp(argv[optind], "promote") == 0)
-               ctl_command = PROMOTE_COMMAND;
-           else if (strcmp(argv[optind], "logrotate") == 0)
-               ctl_command = LOGROTATE_COMMAND;
-           else if (strcmp(argv[optind], "kill") == 0)
-           {
-               if (argc - optind < 3)
-               {
-                   write_stderr(_("%s: missing arguments for kill mode\n"), progname);
-                   do_advice();
-                   exit(1);
-               }
-               ctl_command = KILL_COMMAND;
-               set_sig(argv[++optind]);
-               killproc = atol(argv[++optind]);
-           }
+           ctl_command = KILL_COMMAND;
+           set_sig(argv[++optind]);
+           killproc = atol(argv[++optind]);
+       }
 #ifdef WIN32
-           else if (strcmp(argv[optind], "register") == 0)
-               ctl_command = REGISTER_COMMAND;
-           else if (strcmp(argv[optind], "unregister") == 0)
-               ctl_command = UNREGISTER_COMMAND;
-           else if (strcmp(argv[optind], "runservice") == 0)
-               ctl_command = RUN_AS_SERVICE_COMMAND;
+       else if (strcmp(argv[optind], "register") == 0)
+           ctl_command = REGISTER_COMMAND;
+       else if (strcmp(argv[optind], "unregister") == 0)
+           ctl_command = UNREGISTER_COMMAND;
+       else if (strcmp(argv[optind], "runservice") == 0)
+           ctl_command = RUN_AS_SERVICE_COMMAND;
 #endif
-           else
-           {
-               write_stderr(_("%s: unrecognized operation mode \"%s\"\n"), progname, argv[optind]);
-               do_advice();
-               exit(1);
-           }
-           optind++;
+       else
+       {
+           write_stderr(_("%s: unrecognized operation mode \"%s\"\n"), progname, argv[optind]);
+           do_advice();
+           exit(1);
        }
+       optind++;
+   }
+
+   if (optind < argc)
+   {
+       write_stderr(_("%s: too many command-line arguments (first is \"%s\")\n"), progname, argv[optind]);
+       do_advice();
+       exit(1);
    }
 
    if (ctl_command == NO_COMMAND)