Provide a better error message for misplaced dispatch options.

Before this patch, misplacing 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 aforementioned error
message now looks like

	FATAL:  --single must be first argument

The dispatch option parsing code has been refactored for use
wherever ParseLongOption() is called.  Beyond the obvious advantage
of avoiding code duplication, this should prevent similar problems
when new dispatch options are added.  Note that we assume that none
of the dispatch 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 decided that wasn't worth
the added complexity and behavior changes.

Author: Nathan Bossart, Greg Sabino Mullane
Reviewed-by: Greg Sabino Mullane, Peter Eisentraut, Álvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/CAKAnmmJkZtZAiSryho%3DgYpbvC7H-HNjEDAh16F3SoC9LPu8rqQ%40mail.gmail.com

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index d31a67599c9ac116017f5d6f653cd2fbf314b6e1..a5217773ffc1b266d4aca6bca5387e109a8dd8e9 100644 (file)
--- 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_dispatch_option()
+                                * returns DISPATCH_POSTMASTER if it doesn't find a match, so
+                                * error for anything else.
+                                */
+                               if (parse_dispatch_option(optarg) != DISPATCH_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 aea93a0229826fa8d08b866142d9d0735b76bbb1..3acb46bd464a25dc372cfe2480aa46f8c2720f06 100644 (file)
--- 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 DispatchOptionNames[] =
+{
+       [DISPATCH_CHECK] = "check",
+       [DISPATCH_BOOT] = "boot",
+       [DISPATCH_FORKCHILD] = "forkchild",
+       [DISPATCH_DESCRIBE_CONFIG] = "describe-config",
+       [DISPATCH_SINGLE] = "single",
+       /* DISPATCH_POSTMASTER has no name */
+};
+
+StaticAssertDecl(lengthof(DispatchOptionNames) == DISPATCH_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;
+       DispatchOption dispatch_option = DISPATCH_POSTMASTER;
 
        reached_main = true;
 
@@ -179,26 +193,72 @@ 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] == '-')
+               dispatch_option = parse_dispatch_option(&argv[1][2]);
+
+       switch (dispatch_option)
+       {
+               case DISPATCH_CHECK:
+                       BootstrapModeMain(argc, argv, true);
+                       break;
+               case DISPATCH_BOOT:
+                       BootstrapModeMain(argc, argv, false);
+                       break;
+               case DISPATCH_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 DISPATCH_DESCRIBE_CONFIG:
+                       GucInfoMain();
+                       break;
+               case DISPATCH_SINGLE:
+                       PostgresSingleUserMain(argc, argv,
+                                                                  strdup(get_user_name_or_exit(progname)));
+                       break;
+               case DISPATCH_POSTMASTER:
+                       PostmasterMain(argc, argv);
+                       break;
+       }
+
        /* the functions above should not return */
        abort();
 }
 
+/*
+ * Returns the matching DispatchOption value for the given option name.  If no
+ * match is found, DISPATCH_POSTMASTER is returned.
+ */
+DispatchOption
+parse_dispatch_option(const char *name)
+{
+       for (int i = 0; i < lengthof(DispatchOptionNames); i++)
+       {
+               /*
+                * Unlike the other dispatch 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 DISPATCH_FORKCHILD, so skip over it
+                * in that case.
+                */
+               if (i == DISPATCH_FORKCHILD)
+               {
+#ifdef EXEC_BACKEND
+                       if (strncmp(DispatchOptionNames[DISPATCH_FORKCHILD], name,
+                                               strlen(DispatchOptionNames[DISPATCH_FORKCHILD])) == 0)
+                               return DISPATCH_FORKCHILD;
+#endif
+                       continue;
+               }
+
+               if (strcmp(DispatchOptionNames[i], name) == 0)
+                       return (DispatchOption) i;
+       }
 
+       /* no match means this is a postmaster */
+       return DISPATCH_POSTMASTER;
+}
 
 /*
  * Place platform-specific startup hacks here.  This is the right

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 6376d430870a78ec76dd07297628f2c6572e1f17..ce00f4032eea252007bf90ff2cfebaabc03c2eba 100644 (file)
--- 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_dispatch_option()
+                                * returns DISPATCH_POSTMASTER if it doesn't find a match, so
+                                * error for anything else.
+                                */
+                               if (parse_dispatch_option(optarg) != DISPATCH_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 4b985bd0561513c74719d1da03fe5759ac41f9e0..42af7680456ec0053772ca2b93e06fe32941366e 100644 (file)
--- 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_dispatch_option()
+                                * returns DISPATCH_POSTMASTER if it doesn't find a match, so
+                                * error for anything else.
+                                */
+                               if (parse_dispatch_option(optarg) != DISPATCH_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 f05eb1c470b51757ca717adc83a7cf7bf9a08689..24d49a5439e4f32424c3600742a9bff72edfaa82 100644 (file)
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -138,4 +138,21 @@ extern PMChild *FindPostmasterChildByPid(int pid);
  */
 #define MAX_BACKENDS   0x3FFFF
 
+/*
+ * These values correspond to the special must-be-first options for dispatching
+ * to various subprograms.  parse_dispatch_option() can be used to convert an
+ * option name to one of these values.
+ */
+typedef enum DispatchOption
+{
+       DISPATCH_CHECK,
+       DISPATCH_BOOT,
+       DISPATCH_FORKCHILD,
+       DISPATCH_DESCRIBE_CONFIG,
+       DISPATCH_SINGLE,
+       DISPATCH_POSTMASTER,            /* must be last */
+} DispatchOption;
+
+extern DispatchOption parse_dispatch_option(const char *name);
+
 #endif                                                 /* _POSTMASTER_H */

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 2d4c870423acfe401060dd2e1fcbe3b7c1298165..ce33e55bf1d3a8beb19de27fdc174a019a306daa 100644 (file)
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -619,6 +619,7 @@ DirectoryMethodFile
 DisableTimeoutParams
 DiscardMode
 DiscardStmt
+DispatchOption
 DistanceValue
 DistinctExpr
 DoState