From 95af772ac7106d28db0be4505beebcdc5fd8c902 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Tue, 3 Dec 2024 10:59:32 -0600 Subject: [PATCH v5 1/1] Provide a better error message for misplaced subprogram options. Before this patch, specifying a special must-be-first option for dispatching to a subprogram (e.g., postgres -D . --single) would fail with an error like FATAL: --single requires a value This patch adjusts this error to more accurately complain that the special option wasn't listed first. The previous error message now looks like FATAL: --single must be first argument The subprogram parsing code has been refactored for reuse wherever ParseLongOption() is called. Beyond the obvious advantage of avoiding code duplication, this should prevent similar problems from appearing when new subprograms are added. Note that we assume that none of the subprogram option names match another valid command-line argument, such as the name of a configuration parameter. Ideally, we'd remove this must-be-first requirement for these options, but after some investigation, we felt that the added complexity and behavior changes weren't worth it. Author: Nathan Bossart, Greg Sabino Mullane Reviewed-by: Greg Sabino Mullane, Peter Eisentraut Discussion: https://postgr.es/m/CAKAnmmJkZtZAiSryho%3DgYpbvC7H-HNjEDAh16F3SoC9LPu8rqQ%40mail.gmail.com --- src/backend/bootstrap/bootstrap.c | 15 +++++- src/backend/main/main.c | 84 ++++++++++++++++++++++++----- src/backend/postmaster/postmaster.c | 15 +++++- src/backend/tcop/postgres.c | 15 +++++- src/include/postmaster/postmaster.h | 16 ++++++ src/tools/pgindent/typedefs.list | 1 + 6 files changed, 130 insertions(+), 16 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index d31a67599c..37a0838a90 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -224,8 +224,21 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) case 'B': SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV); break; - case 'c': case '-': + + /* + * Error if the user misplaced a special must-be-first option + * for dispatching to a subprogram. parse_subprogram() returns + * SUBPROGRAM_POSTMASTER if it doesn't find a match, so error + * for anything else. + */ + if (parse_subprogram(optarg) != SUBPROGRAM_POSTMASTER) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("--%s must be first argument", optarg))); + + /* FALLTHROUGH */ + case 'c': { char *name, *value; diff --git a/src/backend/main/main.c b/src/backend/main/main.c index aea93a0229..43380bc7ee 100644 --- a/src/backend/main/main.c +++ b/src/backend/main/main.c @@ -43,6 +43,19 @@ const char *progname; static bool reached_main = false; +/* names of special must-be-first options for dispatching to subprograms */ +static const char *const SubprogramNames[] = +{ + [SUBPROGRAM_CHECK] = "check", + [SUBPROGRAM_BOOT] = "boot", + [SUBPROGRAM_FORKCHILD] = "forkchild", + [SUBPROGRAM_DESCRIBE_CONFIG] = "describe-config", + [SUBPROGRAM_SINGLE] = "single", + /* SUBPROGRAM_POSTMASTER has no name */ +}; + +StaticAssertDecl(lengthof(SubprogramNames) == SUBPROGRAM_POSTMASTER, + "array length mismatch"); static void startup_hacks(const char *progname); static void init_locale(const char *categoryname, int category, const char *locale); @@ -57,6 +70,7 @@ int main(int argc, char *argv[]) { bool do_check_root = true; + Subprogram subprogram = SUBPROGRAM_POSTMASTER; reached_main = true; @@ -179,21 +193,36 @@ main(int argc, char *argv[]) * Dispatch to one of various subprograms depending on first argument. */ - if (argc > 1 && strcmp(argv[1], "--check") == 0) - BootstrapModeMain(argc, argv, true); - else if (argc > 1 && strcmp(argv[1], "--boot") == 0) - BootstrapModeMain(argc, argv, false); + if (argc > 1 && argv[1][0] == '-' && argv[1][1] == '-') + subprogram = parse_subprogram(&argv[1][2]); + + switch (subprogram) + { + case SUBPROGRAM_CHECK: + BootstrapModeMain(argc, argv, true); + break; + case SUBPROGRAM_BOOT: + BootstrapModeMain(argc, argv, false); + break; + case SUBPROGRAM_FORKCHILD: #ifdef EXEC_BACKEND - else if (argc > 1 && strncmp(argv[1], "--forkchild", 11) == 0) - SubPostmasterMain(argc, argv); + SubPostmasterMain(argc, argv); +#else + Assert(false); /* should never happen */ #endif - else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0) - GucInfoMain(); - else if (argc > 1 && strcmp(argv[1], "--single") == 0) - PostgresSingleUserMain(argc, argv, - strdup(get_user_name_or_exit(progname))); - else - PostmasterMain(argc, argv); + break; + case SUBPROGRAM_DESCRIBE_CONFIG: + GucInfoMain(); + break; + case SUBPROGRAM_SINGLE: + PostgresSingleUserMain(argc, argv, + strdup(get_user_name_or_exit(progname))); + break; + case SUBPROGRAM_POSTMASTER: + PostmasterMain(argc, argv); + break; + } + /* the functions above should not return */ abort(); } @@ -440,3 +469,32 @@ __ubsan_default_options(void) return getenv("UBSAN_OPTIONS"); } + +Subprogram +parse_subprogram(const char *name) +{ + for (int i = 0; i < lengthof(SubprogramNames); i++) + { + /* + * Unlike the other subprogram options, "forkchild" takes an argument, + * so we just look for the prefix for that one. For non-EXEC_BACKEND + * builds, we never want to return SUBPROGRAM_FORKCHILD, so skip over + * it in that case. + */ + if (i == SUBPROGRAM_FORKCHILD) + { +#ifdef EXEC_BACKEND + if (strncmp(SubprogramNames[SUBPROGRAM_FORKCHILD], name, + strlen(SubprogramNames[SUBPROGRAM_FORKCHILD])) == 0) + return SUBPROGRAM_FORKCHILD; +#endif + continue; + } + + if (strcmp(SubprogramNames[i], name) == 0) + return (Subprogram) i; + } + + /* no match means this is a postmaster */ + return SUBPROGRAM_POSTMASTER; +} diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 6376d43087..048fa7410c 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -589,8 +589,21 @@ PostmasterMain(int argc, char *argv[]) output_config_variable = strdup(optarg); break; - case 'c': case '-': + + /* + * Error if the user misplaced a special must-be-first option + * for dispatching to a subprogram. parse_subprogram() returns + * SUBPROGRAM_POSTMASTER if it doesn't find a match, so error + * for anything else. + */ + if (parse_subprogram(optarg) != SUBPROGRAM_POSTMASTER) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("--%s must be first argument", optarg))); + + /* FALLTHROUGH */ + case 'c': { char *name, *value; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 4b985bd056..b19638f1fc 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -3947,8 +3947,21 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx, /* ignored for consistency with the postmaster */ break; - case 'c': case '-': + + /* + * Error if the user misplaced a special must-be-first option + * for dispatching to a subprogram. parse_subprogram() returns + * SUBPROGRAM_POSTMASTER if it doesn't find a match, so error + * for anything else. + */ + if (parse_subprogram(optarg) != SUBPROGRAM_POSTMASTER) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("--%s must be first argument", optarg))); + + /* FALLTHROUGH */ + case 'c': { char *name, *value; diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h index f05eb1c470..a65392e3f7 100644 --- a/src/include/postmaster/postmaster.h +++ b/src/include/postmaster/postmaster.h @@ -138,4 +138,20 @@ extern PMChild *FindPostmasterChildByPid(int pid); */ #define MAX_BACKENDS 0x3FFFF +/* special must-be-first options for dispatching to various subprograms */ +typedef enum Subprogram +{ + SUBPROGRAM_CHECK, + SUBPROGRAM_BOOT, + SUBPROGRAM_FORKCHILD, + SUBPROGRAM_DESCRIBE_CONFIG, + SUBPROGRAM_SINGLE, + + /* put new subprograms above */ + + SUBPROGRAM_POSTMASTER, +} Subprogram; + +extern Subprogram parse_subprogram(const char *name); + #endif /* _POSTMASTER_H */ diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 2d4c870423..26aa043ff8 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2770,6 +2770,7 @@ SubXactCallback SubXactCallbackItem SubXactEvent SubXactInfo +Subprogram SubqueryScan SubqueryScanPath SubqueryScanState -- 2.39.5 (Apple Git-154)