On Sun, Aug 25, 2024 at 01:14:36PM -0400, Greg Sabino Mullane wrote:
> I'm not happy about making this and the const char[] change their structure
> based on the ifdefs - could we not just leave forkchild in? Their usage is
> already protected by the ifdefs in the calling code.
Here's an attempt at this.
--
nathan
>From 112026b083615be8debed02cb2c68797301b7319 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@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)
