From 6fa7b16416e74e894bd6db0eee38e94cbb77d7ed Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas 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