Replace getopt() with our re-entrant variant in the backend

Some of these probably could continue using non-re-entrant getopt()
even if we start using threads in the future, but it seems better to
make them all anyway, so that we have a clear-cut rule of "no plain
getopt() in the postgres binary".

Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Discussion: https://www.postgresql.org/message-id/d1da5f0e-0d68-47c9-a882-eb22f462752f@iki.fi

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 68a42de0889dd3cb4c3741f3c32fd35dfc9cfb24..38ef683d4c75a81077a8e532331903c03d31595e 100644 (file)
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -32,7 +32,7 @@
 #include "common/link-canary.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
-#include "pg_getopt.h"
+#include "port/pg_getopt_ctx.h"
 #include "postmaster/postmaster.h"
 #include "storage/bufpage.h"
 #include "storage/fd.h"
@@ -236,6 +236,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 {
        int                     i;
        char       *progname = argv[0];
+       pg_getopt_ctx optctx;
        int                     flag;
        char       *userDoption = NULL;
        uint32          bootstrap_data_checksum_version = 0;    /* No checksum */
@@ -255,12 +256,13 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
        argv++;
        argc--;
 
-       while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:X:-:")) != -1)
+       pg_getopt_start(&optctx, argc, argv, "B:c:d:D:Fkr:X:-:");
+       while ((flag = pg_getopt_next(&optctx)) != -1)
        {
                switch (flag)
                {
                        case 'B':
-                               SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("shared_buffers", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
                        case '-':
 
@@ -270,10 +272,10 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
                                 * returns DISPATCH_POSTMASTER if it doesn't find a match, so
                                 * error for anything else.
                                 */
-                               if (parse_dispatch_option(optarg) != DISPATCH_POSTMASTER)
+                               if (parse_dispatch_option(optctx.optarg) != DISPATCH_POSTMASTER)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_SYNTAX_ERROR),
-                                                        errmsg("--%s must be first argument", optarg)));
+                                                        errmsg("--%s must be first argument", optctx.optarg)));
 
                                pg_fallthrough;
                        case 'c':
@@ -281,19 +283,19 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
                                        char       *name,
                                                           *value;
 
-                                       ParseLongOption(optarg, &name, &value);
+                                       ParseLongOption(optctx.optarg, &name, &value);
                                        if (!value)
                                        {
                                                if (flag == '-')
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("--%s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                                else
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("-c %s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                        }
 
                                        SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
@@ -302,14 +304,14 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
                                        break;
                                }
                        case 'D':
-                               userDoption = pstrdup(optarg);
+                               userDoption = pstrdup(optctx.optarg);
                                break;
                        case 'd':
                                {
                                        /* Turn on debugging for the bootstrap process. */
                                        char       *debugstr;
 
-                                       debugstr = psprintf("debug%s", optarg);
+                                       debugstr = psprintf("debug%s", optctx.optarg);
                                        SetConfigOption("log_min_messages", debugstr,
                                                                        PGC_POSTMASTER, PGC_S_ARGV);
                                        SetConfigOption("client_min_messages", debugstr,
@@ -324,10 +326,10 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
                                bootstrap_data_checksum_version = PG_DATA_CHECKSUM_VERSION;
                                break;
                        case 'r':
-                               strlcpy(OutputFileName, optarg, MAXPGPATH);
+                               strlcpy(OutputFileName, optctx.optarg, MAXPGPATH);
                                break;
                        case 'X':
-                               SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+                               SetConfigOption("wal_segment_size", optctx.optarg, PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
                                break;
                        default:
                                write_stderr("Try \"%s --help\" for more information.\n",
@@ -337,7 +339,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
                }
        }
 
-       if (argc != optind)
+       if (argc != optctx.optind)
        {
                write_stderr("%s: invalid command-line arguments\n", progname);
                proc_exit(1);

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3fac46c402b20490989baa85b0fb38166342b694..abf0c97569e490bbb870fb509d5c964b1247d80f 100644 (file)
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -97,9 +97,9 @@
 #include "lib/ilist.h"
 #include "libpq/libpq.h"
 #include "libpq/pqsignal.h"
-#include "pg_getopt.h"
 #include "pgstat.h"
 #include "port/pg_bswap.h"
+#include "port/pg_getopt_ctx.h"
 #include "postmaster/autovacuum.h"
 #include "postmaster/bgworker_internals.h"
 #include "postmaster/pgarch.h"
@@ -492,6 +492,7 @@ HANDLE              PostmasterHandle;
 void
 PostmasterMain(int argc, char *argv[])
 {
+       pg_getopt_ctx optctx;
        int                     opt;
        int                     status;
        char       *userDoption = NULL;
@@ -588,19 +589,19 @@ PostmasterMain(int argc, char *argv[])
         */
        InitializeGUCOptions();
 
-       opterr = 1;
-
        /*
         * Parse command-line options.  CAUTION: keep this in sync with
         * tcop/postgres.c (the option sets should not conflict) and with the
         * common help() function in main/main.c.
         */
-       while ((opt = getopt(argc, argv, "B:bC:c:D:d:EeFf:h:ijk:lN:OPp:r:S:sTt:W:-:")) != -1)
+       pg_getopt_start(&optctx, argc, argv, "B:bC:c:D:d:EeFf:h:ijk:lN:OPp:r:S:sTt:W:-:");
+       optctx.opterr = 1;
+       while ((opt = pg_getopt_next(&optctx)) != -1)
        {
                switch (opt)
                {
                        case 'B':
-                               SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("shared_buffers", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'b':
@@ -609,7 +610,7 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'C':
-                               output_config_variable = strdup(optarg);
+                               output_config_variable = strdup(optctx.optarg);
                                break;
 
                        case '-':
@@ -620,10 +621,10 @@ PostmasterMain(int argc, char *argv[])
                                 * returns DISPATCH_POSTMASTER if it doesn't find a match, so
                                 * error for anything else.
                                 */
-                               if (parse_dispatch_option(optarg) != DISPATCH_POSTMASTER)
+                               if (parse_dispatch_option(optctx.optarg) != DISPATCH_POSTMASTER)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_SYNTAX_ERROR),
-                                                        errmsg("--%s must be first argument", optarg)));
+                                                        errmsg("--%s must be first argument", optctx.optarg)));
 
                                pg_fallthrough;
                        case 'c':
@@ -631,19 +632,19 @@ PostmasterMain(int argc, char *argv[])
                                        char       *name,
                                                           *value;
 
-                                       ParseLongOption(optarg, &name, &value);
+                                       ParseLongOption(optctx.optarg, &name, &value);
                                        if (!value)
                                        {
                                                if (opt == '-')
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("--%s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                                else
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("-c %s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                        }
 
                                        SetConfigOption(name, value, PGC_POSTMASTER, PGC_S_ARGV);
@@ -653,11 +654,11 @@ PostmasterMain(int argc, char *argv[])
                                }
 
                        case 'D':
-                               userDoption = strdup(optarg);
+                               userDoption = strdup(optctx.optarg);
                                break;
 
                        case 'd':
-                               set_debug_options(atoi(optarg), PGC_POSTMASTER, PGC_S_ARGV);
+                               set_debug_options(atoi(optctx.optarg), PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'E':
@@ -673,16 +674,16 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'f':
-                               if (!set_plan_disabling_options(optarg, PGC_POSTMASTER, PGC_S_ARGV))
+                               if (!set_plan_disabling_options(optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV))
                                {
                                        write_stderr("%s: invalid argument for option -f: \"%s\"\n",
-                                                                progname, optarg);
+                                                                progname, optctx.optarg);
                                        ExitPostmaster(1);
                                }
                                break;
 
                        case 'h':
-                               SetConfigOption("listen_addresses", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("listen_addresses", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'i':
@@ -694,7 +695,7 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'k':
-                               SetConfigOption("unix_socket_directories", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("unix_socket_directories", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'l':
@@ -702,7 +703,7 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'N':
-                               SetConfigOption("max_connections", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("max_connections", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'O':
@@ -714,7 +715,7 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'p':
-                               SetConfigOption("port", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("port", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 'r':
@@ -722,7 +723,7 @@ PostmasterMain(int argc, char *argv[])
                                break;
 
                        case 'S':
-                               SetConfigOption("work_mem", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("work_mem", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        case 's':
@@ -740,7 +741,7 @@ PostmasterMain(int argc, char *argv[])
 
                        case 't':
                                {
-                                       const char *tmp = get_stats_option_name(optarg);
+                                       const char *tmp = get_stats_option_name(optctx.optarg);
 
                                        if (tmp)
                                        {
@@ -749,14 +750,14 @@ PostmasterMain(int argc, char *argv[])
                                        else
                                        {
                                                write_stderr("%s: invalid argument for option -t: \"%s\"\n",
-                                                                        progname, optarg);
+                                                                        progname, optctx.optarg);
                                                ExitPostmaster(1);
                                        }
                                        break;
                                }
 
                        case 'W':
-                               SetConfigOption("post_auth_delay", optarg, PGC_POSTMASTER, PGC_S_ARGV);
+                               SetConfigOption("post_auth_delay", optctx.optarg, PGC_POSTMASTER, PGC_S_ARGV);
                                break;
 
                        default:
@@ -769,10 +770,10 @@ PostmasterMain(int argc, char *argv[])
        /*
         * Postmaster accepts no non-option switch arguments.
         */
-       if (optind < argc)
+       if (optctx.optind < argc)
        {
                write_stderr("%s: invalid argument: \"%s\"\n",
-                                        progname, argv[optind]);
+                                        progname, argv[optctx.optind]);
                write_stderr("Try \"%s --help\" for more information.\n",
                                         progname);
                ExitPostmaster(1);
@@ -868,15 +869,6 @@ PostmasterMain(int argc, char *argv[])
                ExitPostmaster(1);
        }
 
-       /*
-        * Now that we are done processing the postmaster arguments, reset
-        * getopt(3) library so that it will work correctly in subprocesses.
-        */
-       optind = 1;
-#ifdef HAVE_INT_OPTRESET
-       optreset = 1;                           /* some systems need this too */
-#endif
-
        /* For debugging: display postmaster environment */
        if (message_level_is_interesting(DEBUG3))
        {

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 04f4ae116a5be92f6de84667898bedbd755d462f..10be60011ad9f66c6f6c2998d390f61240c925cc 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -51,9 +51,9 @@
 #include "optimizer/optimizer.h"
 #include "parser/analyze.h"
 #include "parser/parser.h"
-#include "pg_getopt.h"
 #include "pg_trace.h"
 #include "pgstat.h"
+#include "port/pg_getopt_ctx.h"
 #include "postmaster/interrupt.h"
 #include "postmaster/postmaster.h"
 #include "replication/logicallauncher.h"
@@ -3838,6 +3838,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
        int                     errs = 0;
        GucSource       gucsource;
        int                     flag;
+       pg_getopt_ctx optctx;
 
        if (secure)
        {
@@ -3855,27 +3856,26 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                gucsource = PGC_S_CLIENT;       /* switches came from client */
        }
 
-#ifdef HAVE_INT_OPTERR
+       /*
+        * Parse command-line options.  CAUTION: keep this in sync with
+        * postmaster/postmaster.c (the option sets should not conflict) and with
+        * the common help() function in main/main.c.
+        */
+       pg_getopt_start(&optctx, argc, argv, "B:bC:c:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:v:W:-:");
 
        /*
         * Turn this off because it's either printed to stderr and not the log
         * where we'd want it, or argv[0] is now "--single", which would make for
         * a weird error message.  We print our own error message below.
         */
-       opterr = 0;
-#endif
+       optctx.opterr = 0;
 
-       /*
-        * Parse command-line options.  CAUTION: keep this in sync with
-        * postmaster/postmaster.c (the option sets should not conflict) and with
-        * the common help() function in main/main.c.
-        */
-       while ((flag = getopt(argc, argv, "B:bC:c:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:v:W:-:")) != -1)
+       while ((flag = pg_getopt_next(&optctx)) != -1)
        {
                switch (flag)
                {
                        case 'B':
-                               SetConfigOption("shared_buffers", optarg, ctx, gucsource);
+                               SetConfigOption("shared_buffers", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 'b':
@@ -3896,10 +3896,10 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                 * returns DISPATCH_POSTMASTER if it doesn't find a match, so
                                 * error for anything else.
                                 */
-                               if (parse_dispatch_option(optarg) != DISPATCH_POSTMASTER)
+                               if (parse_dispatch_option(optctx.optarg) != DISPATCH_POSTMASTER)
                                        ereport(ERROR,
                                                        (errcode(ERRCODE_SYNTAX_ERROR),
-                                                        errmsg("--%s must be first argument", optarg)));
+                                                        errmsg("--%s must be first argument", optctx.optarg)));
 
                                pg_fallthrough;
                        case 'c':
@@ -3907,19 +3907,19 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                        char       *name,
                                                           *value;
 
-                                       ParseLongOption(optarg, &name, &value);
+                                       ParseLongOption(optctx.optarg, &name, &value);
                                        if (!value)
                                        {
                                                if (flag == '-')
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("--%s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                                else
                                                        ereport(ERROR,
                                                                        (errcode(ERRCODE_SYNTAX_ERROR),
                                                                         errmsg("-c %s requires a value",
-                                                                                       optarg)));
+                                                                                       optctx.optarg)));
                                        }
                                        SetConfigOption(name, value, ctx, gucsource);
                                        pfree(name);
@@ -3929,11 +3929,11 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 
                        case 'D':
                                if (secure)
-                                       userDoption = strdup(optarg);
+                                       userDoption = strdup(optctx.optarg);
                                break;
 
                        case 'd':
-                               set_debug_options(atoi(optarg), ctx, gucsource);
+                               set_debug_options(atoi(optctx.optarg), ctx, gucsource);
                                break;
 
                        case 'E':
@@ -3950,12 +3950,12 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                break;
 
                        case 'f':
-                               if (!set_plan_disabling_options(optarg, ctx, gucsource))
+                               if (!set_plan_disabling_options(optctx.optarg, ctx, gucsource))
                                        errs++;
                                break;
 
                        case 'h':
-                               SetConfigOption("listen_addresses", optarg, ctx, gucsource);
+                               SetConfigOption("listen_addresses", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 'i':
@@ -3968,7 +3968,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                break;
 
                        case 'k':
-                               SetConfigOption("unix_socket_directories", optarg, ctx, gucsource);
+                               SetConfigOption("unix_socket_directories", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 'l':
@@ -3976,7 +3976,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                break;
 
                        case 'N':
-                               SetConfigOption("max_connections", optarg, ctx, gucsource);
+                               SetConfigOption("max_connections", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 'n':
@@ -3992,17 +3992,17 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                break;
 
                        case 'p':
-                               SetConfigOption("port", optarg, ctx, gucsource);
+                               SetConfigOption("port", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 'r':
                                /* send output (stdout and stderr) to the given file */
                                if (secure)
-                                       strlcpy(OutputFileName, optarg, MAXPGPATH);
+                                       strlcpy(OutputFileName, optctx.optarg, MAXPGPATH);
                                break;
 
                        case 'S':
-                               SetConfigOption("work_mem", optarg, ctx, gucsource);
+                               SetConfigOption("work_mem", optctx.optarg, ctx, gucsource);
                                break;
 
                        case 's':
@@ -4015,7 +4015,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 
                        case 't':
                                {
-                                       const char *tmp = get_stats_option_name(optarg);
+                                       const char *tmp = get_stats_option_name(optctx.optarg);
 
                                        if (tmp)
                                                SetConfigOption(tmp, "true", ctx, gucsource);
@@ -4034,11 +4034,11 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
                                 * standalone backend.
                                 */
                                if (secure)
-                                       FrontendProtocol = (ProtocolVersion) atoi(optarg);
+                                       FrontendProtocol = (ProtocolVersion) atoi(optctx.optarg);
                                break;
 
                        case 'W':
-                               SetConfigOption("post_auth_delay", optarg, ctx, gucsource);
+                               SetConfigOption("post_auth_delay", optctx.optarg, ctx, gucsource);
                                break;
 
                        default:
@@ -4053,36 +4053,27 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
        /*
         * Optional database name should be there only if *dbname is NULL.
         */
-       if (!errs && dbname && *dbname == NULL && argc - optind >= 1)
-               *dbname = strdup(argv[optind++]);
+       if (!errs && dbname && *dbname == NULL && argc - optctx.optind >= 1)
+               *dbname = strdup(argv[optctx.optind++]);
 
-       if (errs || argc != optind)
+       if (errs || argc != optctx.optind)
        {
                if (errs)
-                       optind--;                       /* complain about the previous argument */
+                       optctx.optind--;        /* complain about the previous argument */
 
                /* spell the error message a bit differently depending on context */
                if (IsUnderPostmaster)
                        ereport(FATAL,
                                        errcode(ERRCODE_SYNTAX_ERROR),
-                                       errmsg("invalid command-line argument for server process: %s", argv[optind]),
+                                       errmsg("invalid command-line argument for server process: %s", argv[optctx.optind]),
                                        errhint("Try \"%s --help\" for more information.", progname));
                else
                        ereport(FATAL,
                                        errcode(ERRCODE_SYNTAX_ERROR),
                                        errmsg("%s: invalid command-line argument: %s",
-                                                  progname, argv[optind]),
+                                                  progname, argv[optctx.optind]),
                                        errhint("Try \"%s --help\" for more information.", progname));
        }
-
-       /*
-        * Reset getopt(3) library so that it will work correctly in subprocesses
-        * or when this function is called a second time with another array.
-        */
-       optind = 1;
-#ifdef HAVE_INT_OPTRESET
-       optreset = 1;                           /* some systems need this too */
-#endif
 }
 
 

diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index 51dce24947a8cf62d5da9c15ebd23e916001fe9c..cde10dd59d2feced70c1dd5e843096746c5626b6 100644 (file)
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -234,7 +234,8 @@ save_ps_display_args(int argc, char **argv)
         * into the argv array, and will get horribly confused when it is
         * re-called to analyze a subprocess' argument string if the argv storage
         * has been clobbered meanwhile.  Other platforms have other dependencies
-        * on argv[].
+        * on argv[]. (We use custom pg_getopt_start/next() functions nowadays
+        * that don't do that, but those other dependencies might still exist.)
         */
        {
                char      **new_argv;