Better error message when --single is not the first arg to postgres executable

From 44f8747a75197b3842c41f77503fa8389ba8db3e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 17 Jun 2024 21:20:13 -0500
Subject: [PATCH 1/1] allow --single, etc. in any order

---
 src/backend/bootstrap/bootstrap.c |  12 ++--
 src/backend/main/main.c           | 101 ++++++++++++++++++++----------
 src/backend/tcop/postgres.c       |  15 ++---
 3 files changed, 77 insertions(+), 51 deletions(-)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 986f6f1d9c..53011b4300 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -210,13 +210,6 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 	/* Set defaults, to be overridden by explicit options below */
 	InitializeGUCOptions();
 
-	/* an initial --boot or --check should be present */
-	Assert(argc > 1
-		   && (strcmp(argv[1], "--boot") == 0
-			   || strcmp(argv[1], "--check") == 0));
-	argv++;
-	argc--;
-
 	while ((flag = getopt(argc, argv, "B:c:d:D:Fkr:X:-:")) != -1)
 	{
 		switch (flag)
@@ -230,6 +223,11 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 					char	   *name,
 							   *value;
 
+					/* --boot and --check were already processed in main() */
+					if (strcmp(optarg, "boot") == 0 ||
+						strcmp(optarg, "check") == 0)
+						break;
+
 					ParseLongOption(optarg, &name, &value);
 					if (!value)
 					{
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index bfd0c5ed65..b893a9f298 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -57,7 +57,17 @@ static void check_root(const char *progname);
 int
 main(int argc, char *argv[])
 {
-	bool		do_check_root = true;
+	bool		check = false;
+	bool		boot = false;
+#ifdef EXEC_BACKEND
+	bool		forkchild = false;
+#endif
+	bool		describe_config = false;
+	bool		single = false;
+	bool		show_help = false;
+	bool		version = false;
+	bool		check_guc = false;
+	int			modes_set = 0;
 
 	reached_main = true;
 
@@ -136,61 +146,86 @@ main(int argc, char *argv[])
 	 */
 	unsetenv("LC_ALL");
 
+	for (int i = 1; i < argc; i++)
+	{
+		modes_set++;
+
+		if (strcmp(argv[i], "--check") == 0)
+			check = true;
+		else if (strcmp(argv[i], "--boot") == 0)
+			boot = true;
+#ifdef EXEC_BACKEND
+		else if (strncmp(argv[i], "--forkchild", 11) == 0)
+			forkchild = true;
+#endif
+		else if (strcmp(argv[i], "--describe-config") == 0)
+			describe_config = true;
+		else if (strcmp(argv[i], "--single") == 0)
+			single = true;
+		else if (strcmp(argv[i], "--help") == 0 || strcmp(argv[i], "-?") == 0)
+			show_help = true;
+		else if (strcmp(argv[i], "--version") == 0 || strcmp(argv[i], "-V") == 0)
+			version = true;
+		else if (strcmp(argv[i], "-C") == 0)
+			check_guc = true;
+		else
+			modes_set--;
+	}
+
+	if (modes_set > 1)
+	{
+		/* hack to avoid assertion failure in proc_exit() */
+		MyProcPid = getpid();
+
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("multiple server modes set"),
+				 errdetail("Only one of %s may be set.",
+#ifdef EXEC_BACKEND
+						   "--check, --boot, --forkchild, --describe-config, --single, --help/-?, --version/-V, -C")));
+#else
+						   "--check, --boot, --describe-config, --single, --help/-?, --version/-V, -C")));
+#endif
+	}
+
 	/*
 	 * Catch standard options before doing much else, in particular before we
 	 * insist on not being root.
 	 */
-	if (argc > 1)
+	if (show_help)
 	{
-		if (strcmp(argv[1], "--help") == 0 || strcmp(argv[1], "-?") == 0)
-		{
-			help(progname);
-			exit(0);
-		}
-		if (strcmp(argv[1], "--version") == 0 || strcmp(argv[1], "-V") == 0)
-		{
-			fputs(PG_BACKEND_VERSIONSTR, stdout);
-			exit(0);
-		}
+		help(progname);
+		exit(0);
+	}
 
-		/*
-		 * In addition to the above, we allow "--describe-config" and "-C var"
-		 * to be called by root.  This is reasonably safe since these are
-		 * read-only activities.  The -C case is important because pg_ctl may
-		 * try to invoke it while still holding administrator privileges on
-		 * Windows.  Note that while -C can normally be in any argv position,
-		 * if you want to bypass the root check you must put it first.  This
-		 * reduces the risk that we might misinterpret some other mode's -C
-		 * switch as being the postmaster/postgres one.
-		 */
-		if (strcmp(argv[1], "--describe-config") == 0)
-			do_check_root = false;
-		else if (argc > 2 && strcmp(argv[1], "-C") == 0)
-			do_check_root = false;
+	if (version)
+	{
+		fputs(PG_BACKEND_VERSIONSTR, stdout);
+		exit(0);
 	}
 
 	/*
 	 * Make sure we are not running as root, unless it's safe for the selected
 	 * option.
 	 */
-	if (do_check_root)
+	if (!describe_config && !check_guc)
 		check_root(progname);
 
 	/*
-	 * Dispatch to one of various subprograms depending on first argument.
+	 * Dispatch to one of various subprograms.
 	 */
 
-	if (argc > 1 && strcmp(argv[1], "--check") == 0)
+	if (check)
 		BootstrapModeMain(argc, argv, true);
-	else if (argc > 1 && strcmp(argv[1], "--boot") == 0)
+	else if (boot)
 		BootstrapModeMain(argc, argv, false);
 #ifdef EXEC_BACKEND
-	else if (argc > 1 && strncmp(argv[1], "--fork", 6) == 0)
+	else if (forkchild)
 		SubPostmasterMain(argc, argv);
 #endif
-	else if (argc > 1 && strcmp(argv[1], "--describe-config") == 0)
+	else if (describe_config)
 		GucInfoMain();
-	else if (argc > 1 && strcmp(argv[1], "--single") == 0)
+	else if (single)
 		PostgresSingleUserMain(argc, argv,
 							   strdup(get_user_name_or_exit(progname)));
 	else
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 45a3794b8e..74f96ce479 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3796,20 +3796,9 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 	int			flag;
 
 	if (secure)
-	{
 		gucsource = PGC_S_ARGV; /* switches came from command line */
-
-		/* Ignore the initial --single argument, if present */
-		if (argc > 1 && strcmp(argv[1], "--single") == 0)
-		{
-			argv++;
-			argc--;
-		}
-	}
 	else
-	{
 		gucsource = PGC_S_CLIENT;	/* switches came from client */
-	}
 
 #ifdef HAVE_INT_OPTERR
 
@@ -3850,6 +3839,10 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
 					char	   *name,
 							   *value;
 
+					/* --single was already processed in main() */
+					if (strcmp(optarg, "single") == 0)
+						break;
+
 					ParseLongOption(optarg, &name, &value);
 					if (!value)
 					{
-- 
2.39.3 (Apple Git-146)

From 5b2a98ad0571e93d789bec4bb9343f1307fc3f53 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 21 Aug 2024 15:39:05 -0500
Subject: [PATCH v3 1/1] better error message when subprogram argument is not
 first

---
 src/backend/main/main.c             | 77 ++++++++++++++++++++++++-----
 src/backend/utils/misc/guc.c        |  6 +++
 src/include/postmaster/postmaster.h | 18 +++++++
 src/tools/pgindent/typedefs.list    |  1 +
 4 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index 4672aab837..ab1d823820 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -44,6 +44,20 @@
 const char *progname;
 static bool reached_main = false;
 
+static const char *const SubprogramNames[] =
+{
+	[SUBPROGRAM_CHECK] = "check",
+	[SUBPROGRAM_BOOT] = "boot",
+#ifdef EXEC_BACKEND
+	[SUBPROGRAM_FORKCHILD] = "forkchild",
+#endif
+	[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);
@@ -58,6 +72,7 @@ int
 main(int argc, char *argv[])
 {
 	bool		do_check_root = true;
+	Subprogram	subprogram = SUBPROGRAM_POSTMASTER;
 
 	reached_main = true;
 
@@ -180,21 +195,34 @@ 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;
 #ifdef EXEC_BACKEND
-	else if (argc > 1 && strncmp(argv[1], "--forkchild", 11) == 0)
-		SubPostmasterMain(argc, argv);
+		case SUBPROGRAM_FORKCHILD:
+			SubPostmasterMain(argc, argv);
+			break;
 #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);
+		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();
 }
@@ -441,3 +469,26 @@ __ubsan_default_options(void)
 
 	return getenv("UBSAN_OPTIONS");
 }
+
+Subprogram
+parse_subprogram(const char *name)
+{
+#ifdef EXEC_BACKEND
+	/*
+	 * Unlike the other subprogram options, "forkchild" takes an argument, so
+	 * we just look for the prefix for that one.
+	 */
+	if (strncmp(SubprogramNames[SUBPROGRAM_FORKCHILD], name,
+				strlen(SubprogramNames[SUBPROGRAM_FORKCHILD])) == 0)
+		return SUBPROGRAM_FORKCHILD;
+#endif
+
+	for (int i = 0; i < lengthof(SubprogramNames); i++)
+	{
+		if (strcmp(SubprogramNames[i], name) == 0)
+			return (Subprogram) i;
+	}
+
+	/* no match means this is a postmaster */
+	return SUBPROGRAM_POSTMASTER;
+}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 13527fc258..efa25c7b9a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -40,6 +40,7 @@
 #include "miscadmin.h"
 #include "parser/scansup.h"
 #include "port/pg_bitutils.h"
+#include "postmaster/postmaster.h"
 #include "storage/fd.h"
 #include "storage/lwlock.h"
 #include "storage/shmem.h"
@@ -6333,6 +6334,11 @@ ParseLongOption(const char *string, char **name, char **value)
 	Assert(name);
 	Assert(value);
 
+	/* parse_subprogram() returns SUBPROGRAM_POSTMASTER if no match */
+	if (unlikely(parse_subprogram(string) != SUBPROGRAM_POSTMASTER))
+		ereport(ERROR,
+				(errmsg("--%s must be first argument", string)));
+
 	equal_pos = strcspn(string, "=");
 
 	if (string[equal_pos] == '=')
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 63c12917cf..d26658c69e 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -92,4 +92,22 @@ extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
  */
 #define MAX_BACKENDS	0x3FFFF
 
+/* special must-be-first options for dispatching to various subprograms */
+typedef enum Subprogram
+{
+	SUBPROGRAM_CHECK,
+	SUBPROGRAM_BOOT,
+#ifdef EXEC_BACKEND
+	SUBPROGRAM_FORKCHILD,
+#endif
+	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 6d424c8918..e7f977a290 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2765,6 +2765,7 @@ SubXactCallback
 SubXactCallbackItem
 SubXactEvent
 SubXactInfo
+Subprogram
 SubqueryScan
 SubqueryScanPath
 SubqueryScanState
-- 
2.39.3 (Apple Git-146)

From 112026b083615be8debed02cb2c68797301b7319 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Wed, 21 Aug 2024 15:39:05 -0500
Subject: [PATCH v4 1/1] better error message when subprogram argument is not
 first

---
 src/backend/main/main.c             | 84 ++++++++++++++++++++++++-----
 src/backend/utils/misc/guc.c        |  6 +++
 src/include/postmaster/postmaster.h | 16 ++++++
 src/tools/pgindent/typedefs.list    |  1 +
 4 files changed, 94 insertions(+), 13 deletions(-)

diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index 4672aab837..24ba012acf 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -44,6 +44,18 @@
 const char *progname;
 static bool reached_main = false;
 
+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);
@@ -58,6 +70,7 @@ int
 main(int argc, char *argv[])
 {
 	bool		do_check_root = true;
+	Subprogram	subprogram = SUBPROGRAM_POSTMASTER;
 
 	reached_main = true;
 
@@ -180,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();
 }
@@ -441,3 +469,33 @@ __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/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 13527fc258..efa25c7b9a 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -40,6 +40,7 @@
 #include "miscadmin.h"
 #include "parser/scansup.h"
 #include "port/pg_bitutils.h"
+#include "postmaster/postmaster.h"
 #include "storage/fd.h"
 #include "storage/lwlock.h"
 #include "storage/shmem.h"
@@ -6333,6 +6334,11 @@ ParseLongOption(const char *string, char **name, char **value)
 	Assert(name);
 	Assert(value);
 
+	/* parse_subprogram() returns SUBPROGRAM_POSTMASTER if no match */
+	if (unlikely(parse_subprogram(string) != SUBPROGRAM_POSTMASTER))
+		ereport(ERROR,
+				(errmsg("--%s must be first argument", string)));
+
 	equal_pos = strcspn(string, "=");
 
 	if (string[equal_pos] == '=')
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 63c12917cf..3cea517ef6 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -92,4 +92,20 @@ extern void SubPostmasterMain(int argc, char *argv[]) pg_attribute_noreturn();
  */
 #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 9e951a9e6f..ec94a02860 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2764,6 +2764,7 @@ SubXactCallback
 SubXactCallbackItem
 SubXactEvent
 SubXactInfo
+Subprogram
 SubqueryScan
 SubqueryScanPath
 SubqueryScanState
-- 
2.39.3 (Apple Git-146)

From 95af772ac7106d28db0be4505beebcdc5fd8c902 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
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)

From bb5219251479ef8f54df299269ae738ba19ce681 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 3 Dec 2024 13:50:36 -0600
Subject: [PATCH v6 1/1] Provide a better error message for misplaced dispatch
 options.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 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 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, Álvaro Herrera, Tom Lane
Discussion: https://postgr.es/m/CAKAnmmJkZtZAiSryho%3DgYpbvC7H-HNjEDAh16F3SoC9LPu8rqQ%40mail.gmail.com
---
 src/backend/bootstrap/bootstrap.c   | 15 ++++-
 src/backend/main/main.c             | 86 ++++++++++++++++++++++++-----
 src/backend/postmaster/postmaster.c | 15 ++++-
 src/backend/tcop/postgres.c         | 15 ++++-
 src/include/postmaster/postmaster.h | 17 ++++++
 src/tools/pgindent/typedefs.list    |  1 +
 6 files changed, 133 insertions(+), 16 deletions(-)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index d31a67599c..a5217773ff 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_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 aea93a0229..3acb46bd46 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 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 6376d43087..ce00f4032e 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_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 4b985bd056..42af768045 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_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 f05eb1c470..24d49a5439 100644
--- 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 2d4c870423..ce33e55bf1 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -619,6 +619,7 @@ DirectoryMethodFile
 DisableTimeoutParams
 DiscardMode
 DiscardStmt
+DispatchOption
 DistanceValue
 DistinctExpr
 DoState
-- 
2.39.5 (Apple Git-154)