On 06/06/2024 17:34, Peter Eisentraut wrote:
Additionally, there are non-standard functions that are not
thread-safe, such as getopt_long().
getopt_long() is not used in the server, only in client programs. The
server binary does actually accept a few "long" arguments, like --single
and --describe-config, but it has special code to handle them and
doesn't use getopt_long(). So we can leave getopt_long() alone for now.
- getopt()
This needs a custom replacement. (There is no getopt_r() because
programs usually don't call getopt() after startup.)
(Note: This is also called during session startup, not only during
initial postmaster start. So we definitely need something here, if we
want to, like, start more than one session concurrently.)
Here's a patch for a thread-safe version of getopt(). I implemented it
as two functions, pg_getopt_start() and pg_getopt_next(). Since there's
no standard to follow, we have freedom on how to implement it, and IMHO
that feels more natural than the single getopt() function. I took the
implementation from the getopt() replacement we already had for
platforms that don't have getopt(), moving all the global variables it
used to a struct.
The last patch attached replaces all calls in the server to use the new
variant, but leaves all the calls in client programs alone. I considered
changing the client programs as well, but there's no immediate need, and
it seems nice to use OS functions when possible.
(The first patch fixes a little harmless bug in get_stats_option_name()
that's gone unnoticed since 2006 but got in the way now.)
--
Heikki Linnakangas
Neon (https://neon.tech)
From 3b02801188b7961d01d7c47fc9b923034bfbb822 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Mon, 19 May 2025 23:15:52 +0300
Subject: [PATCH 1/3] Fix latent bug in get_stats_option_name()
The function is supposed to look at the passed in 'arg' argument, but
peeks at the 'optarg' global variable that's part of getopt()
instead. It happened to work anyway, because all callers passed
'optarg' as the argument.
---
src/backend/tcop/postgres.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 1ae51b1b391..f069acb9ee2 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3757,9 +3757,9 @@ get_stats_option_name(const char *arg)
switch (arg[0])
{
case 'p':
- if (optarg[1] == 'a') /* "parser" */
+ if (arg[1] == 'a') /* "parser" */
return "log_parser_stats";
- else if (optarg[1] == 'l') /* "planner" */
+ else if (arg[1] == 'l') /* "planner" */
return "log_planner_stats";
break;
--
2.39.5
From 8b8f62850231148e4471e421288fa0d175e48039 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Mon, 19 May 2025 22:33:59 +0300
Subject: [PATCH 2/3] Invent custom pg_getopt_ctx that is thread-safe
The standard getopt(3) function is not re-entrant nor
thread-safe. That's OK for current usage, but it's one more little
thing we need to change in order to make the server multi-threaded.
There's no standard getopt_r() function on any platform, because
command line arguments are usually parsed early when you start a
program, usually before launching any threads, so there's not much
need for it. However, we call it at backend startup to parse options
from the startup packet. We are therefore free to define our own.
The pg_getopt_start/next() implementation is based on the old getopt
implementation, I just gathered all the state variables to a struct.
The non-re-entrant getopt() function is now a wrapper around the
custom re-entrant variant, on platforms that don't have getopt(3).
getopt_long() is not used in the server, so we don't need to provide a
re-entrant variant of that.
---
src/include/port/pg_getopt_ctx.h | 29 +++++++
src/port/Makefile | 1 +
src/port/getopt.c | 91 +++++----------------
src/port/meson.build | 1 +
src/port/pg_getopt_ctx.c | 136 +++++++++++++++++++++++++++++++
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 189 insertions(+), 70 deletions(-)
create mode 100644 src/include/port/pg_getopt_ctx.h
create mode 100644 src/port/pg_getopt_ctx.c
diff --git a/src/include/port/pg_getopt_ctx.h b/src/include/port/pg_getopt_ctx.h
new file mode 100644
index 00000000000..5066915d259
--- /dev/null
+++ b/src/include/port/pg_getopt_ctx.h
@@ -0,0 +1,29 @@
+/*
+ * Re-entrant version of the standard getopt(3) function.
+ *
+ * Portions Copyright (c) 2025, PostgreSQL Global Development Group
+ *
+ * src/include/port/pg_getopt_ctx.h
+ */
+#ifndef PG_GETOPT_CTX_H
+#define PG_GETOPT_CTX_H
+
+typedef struct
+{
+ int nargc;
+ char *const *nargv;
+ const char *ostr;
+
+ char *optarg;
+ int optind;
+ int opterr;
+ int optopt;
+
+ /* internal state */
+ char *place;
+} pg_getopt_ctx;
+
+extern void pg_getopt_start(pg_getopt_ctx *ctx, int nargc, char *const *nargv, const char *ostr);
+extern int pg_getopt_next(pg_getopt_ctx *ctx);
+
+#endif /* PG_GETOPT_CTX_H */
diff --git a/src/port/Makefile b/src/port/Makefile
index 4274949dfa4..5aa5867b5ea 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -44,6 +44,7 @@ OBJS = \
noblock.o \
path.o \
pg_bitutils.o \
+ pg_getopt_ctx.o \
pg_localeconv_r.o \
pg_numa.o \
pg_popcount_aarch64.o \
diff --git a/src/port/getopt.c b/src/port/getopt.c
index 655fef3b0c7..34a60d5f32d 100644
--- a/src/port/getopt.c
+++ b/src/port/getopt.c
@@ -32,11 +32,7 @@
#include "c.h"
#include "pg_getopt.h"
-
-#if defined(LIBC_SCCS) && !defined(lint)
-static char sccsid[] = "@(#)getopt.c 8.3 (Berkeley) 4/27/95";
-#endif /* LIBC_SCCS and not lint */
-
+#include "port/pg_getopt_ctx.h"
/*
* On OpenBSD and some versions of Solaris, opterr and friends are defined in
@@ -54,84 +50,39 @@ char *optarg; /* argument associated with option */
#endif
-#define BADCH (int)'?'
-#define BADARG (int)':'
-#define EMSG ""
-
/*
* getopt
* Parse argc/argv argument vector.
*
+ * We use the re-entrant pg_getopt_ctx() function under the hood, but expose the
+ * standard non re-entrant API.
+ *
* 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
+ * (Internally, this means we must be sure to reset "active" before
* returning -1.)
*/
int
getopt(int nargc, char *const *nargv, const char *ostr)
{
- static char *place = EMSG; /* option letter processing */
- char *oli; /* option letter list index */
+ static bool active = false;
+ static pg_getopt_ctx ctx;
+ int result;
- if (!*place)
- { /* update scanning pointer */
- if (optind >= nargc || *(place = nargv[optind]) != '-')
- {
- place = EMSG;
- return -1;
- }
- if (place[1] && *++place == '-' && place[1] == '\0')
- { /* found "--" */
- ++optind;
- place = EMSG;
- return -1;
- }
- } /* option letter okay? */
- if ((optopt = (int) *place++) == (int) ':' ||
- !(oli = strchr(ostr, optopt)))
+ if (!active)
{
- /*
- * if the user didn't specify '-' as an option, assume it means -1.
- */
- if (optopt == (int) '-')
- {
- place = EMSG;
- return -1;
- }
- if (!*place)
- ++optind;
- if (opterr && *ostr != ':')
- (void) fprintf(stderr,
- "illegal option -- %c\n", optopt);
- return BADCH;
+ pg_getopt_start(&ctx, nargc, nargv, ostr);
+ ctx.opterr = opterr;
+ active = true;
}
- if (*++oli != ':')
- { /* don't need argument */
- optarg = NULL;
- if (!*place)
- ++optind;
- }
- else
- { /* need an argument */
- if (*place) /* no white space */
- optarg = place;
- else if (nargc <= ++optind)
- { /* no arg */
- place = EMSG;
- if (*ostr == ':')
- return BADARG;
- if (opterr)
- (void) fprintf(stderr,
- "option requires an argument -- %c\n",
- optopt);
- return BADCH;
- }
- else
- /* white space */
- optarg = nargv[optind];
- place = EMSG;
- ++optind;
- }
- return optopt; /* dump back option letter */
+
+ result = pg_getopt_next(&ctx);
+ opterr = ctx.opterr;
+ optind = ctx.optind;
+ optopt = ctx.optopt;
+ optarg = ctx.optarg;
+ if (result == -1)
+ active = false;
+ return result;
}
diff --git a/src/port/meson.build b/src/port/meson.build
index fc7b059fee5..a6ec9c82f3a 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -7,6 +7,7 @@ pgport_sources = [
'noblock.c',
'path.c',
'pg_bitutils.c',
+ 'pg_getopt_ctx.c',
'pg_localeconv_r.c',
'pg_numa.c',
'pg_popcount_aarch64.c',
diff --git a/src/port/pg_getopt_ctx.c b/src/port/pg_getopt_ctx.c
new file mode 100644
index 00000000000..c125f47813f
--- /dev/null
+++ b/src/port/pg_getopt_ctx.c
@@ -0,0 +1,136 @@
+/*-------------------------------------------------------------------------
+ *
+ * pg_getopt_ctx.c
+ * Thread-safe implementation of getopt()
+ *
+ * Copyright (c) 1987, 1993, 1994
+ * The Regents of the University of California. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ *
+ * IDENTIFICATION
+ * src/port/pg_getopt_ctx.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "c.h"
+
+#include "port/pg_getopt_ctx.h"
+
+#define BADCH (int)'?'
+#define BADARG (int)':'
+#define EMSG ""
+
+/*
+ * Start parsing argc/argv argument vector.
+ *
+ * This is a re-entrant version of the standard library getopt(3) function. To
+ * use, first call pg_getopt_start() to initialize the state, and then call
+ * pg_getopt_next() until it returns -1.
+ */
+void
+pg_getopt_start(pg_getopt_ctx *ctx, int nargc, char *const *nargv, const char *ostr)
+{
+ ctx->nargc = nargc;
+ ctx->nargv = nargv;
+ ctx->ostr = ostr;
+
+ ctx->optind = 1;
+ ctx->optarg = NULL;
+ ctx->opterr = 0; /* Caller may set this after the call */
+ ctx->optopt = 0;
+
+ ctx->place = EMSG; /* option letter processing */
+}
+
+/*
+ * Parse next option in argc/argv argument vector
+ */
+int
+pg_getopt_next(pg_getopt_ctx *ctx)
+{
+ char *oli; /* option letter list index */
+
+ if (!*ctx->place)
+ { /* update scanning pointer */
+ if (ctx->optind >= ctx->nargc || *(ctx->place = ctx->nargv[ctx->optind]) != '-')
+ {
+ ctx->place = EMSG;
+ return -1;
+ }
+ if (ctx->place[1] && *++ctx->place == '-' && ctx->place[1] == '\0')
+ { /* found "--" */
+ ++ctx->optind;
+ ctx->place = EMSG;
+ return -1;
+ }
+ } /* option letter okay? */
+ if ((ctx->optopt = (int) *ctx->place++) == (int) ':' ||
+ !(oli = strchr(ctx->ostr, ctx->optopt)))
+ {
+ /*
+ * if the user didn't specify '-' as an option, assume it means -1.
+ */
+ if (ctx->optopt == (int) '-')
+ {
+ ctx->place = EMSG;
+ return -1;
+ }
+ if (!*ctx->place)
+ ++ctx->optind;
+ if (ctx->opterr && *ctx->ostr != ':')
+ (void) fprintf(stderr,
+ "illegal option -- %c\n", ctx->optopt);
+ return BADCH;
+ }
+ if (*++oli != ':')
+ { /* don't need argument */
+ ctx->optarg = NULL;
+ if (!*ctx->place)
+ ++ctx->optind;
+ }
+ else
+ { /* need an argument */
+ if (*ctx->place) /* no white space */
+ ctx->optarg = ctx->place;
+ else if (ctx->nargc <= ++ctx->optind)
+ { /* no arg */
+ ctx->place = EMSG;
+ if (*ctx->ostr == ':')
+ return BADARG;
+ if (ctx->opterr)
+ (void) fprintf(stderr,
+ "option requires an argument -- %c\n",
+ ctx->optopt);
+ return BADCH;
+ }
+ else
+ /* white space */
+ ctx->optarg = ctx->nargv[ctx->optind];
+ ctx->place = EMSG;
+ ++ctx->optind;
+ }
+ return ctx->optopt; /* dump back option letter */
+}
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 9ea573fae21..49ac9a1e47c 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3830,6 +3830,7 @@ pg_enc2name
pg_encname
pg_fe_sasl_mech
pg_funcptr_t
+pg_getopt_ctx
pg_gssinfo
pg_hmac_ctx
pg_hmac_errno
--
2.39.5
From 6fa7b16416e74e894bd6db0eee38e94cbb77d7ed Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakan...@iki.fi>
Date: Mon, 19 May 2025 22:53:45 +0300
Subject: [PATCH 3/3] Replace getopt() with our re-entrant variant in the
backend
---
src/backend/bootstrap/bootstrap.c | 28 ++++++-----
src/backend/postmaster/postmaster.c | 60 ++++++++++------------
src/backend/tcop/postgres.c | 77 +++++++++++++----------------
src/backend/utils/misc/ps_status.c | 3 +-
4 files changed, 77 insertions(+), 91 deletions(-)
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6db864892d0..c9ef5e0f116 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -30,7 +30,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/ipc.h"
@@ -199,6 +199,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 */
@@ -218,12 +219,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 '-':
@@ -233,10 +235,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)));
/* FALLTHROUGH */
case 'c':
@@ -244,19 +246,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);
@@ -265,14 +267,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,
@@ -287,10 +289,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",
@@ -300,7 +302,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 490f7ce3664..6ec5a7cebaa 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -97,8 +97,8 @@
#include "lib/ilist.h"
#include "libpq/libpq.h"
#include "libpq/pqsignal.h"
-#include "pg_getopt.h"
#include "pgstat.h"
+#include "port/pg_getopt_ctx.h"
#include "port/pg_bswap.h"
#include "postmaster/autovacuum.h"
#include "postmaster/bgworker_internals.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)));
/* 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);
@@ -865,15 +866,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 f069acb9ee2..5a2322f33e8 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -50,9 +50,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"
@@ -3799,6 +3799,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
int errs = 0;
GucSource gucsource;
int flag;
+ pg_getopt_ctx optctx;
if (secure)
{
@@ -3816,27 +3817,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':
@@ -3857,10 +3857,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)));
/* FALLTHROUGH */
case 'c':
@@ -3868,19 +3868,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);
@@ -3890,11 +3890,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':
@@ -3911,12 +3911,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':
@@ -3929,7 +3929,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':
@@ -3937,7 +3937,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':
@@ -3953,17 +3953,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':
@@ -3976,7 +3976,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);
@@ -3995,11 +3995,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:
@@ -4014,36 +4014,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 e08b26e8c14..30fe883939a 100644
--- a/src/backend/utils/misc/ps_status.c
+++ b/src/backend/utils/misc/ps_status.c
@@ -218,7 +218,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;
--
2.39.5
