On Wed, Jun 19, 2024 at 10:58:02AM -0500, Nathan Bossart wrote:
> On Wed, Jun 19, 2024 at 05:34:52PM +0200, Peter Eisentraut wrote:
>> I'm not really sure all this here is worth solving. I think requiring
>> things like --single or --boot to be first seems ok, and the alternatives
>> just make things more complicated.
>
> Yeah, I'm fine with doing something more like what Greg originally
> proposed at this point.
Here's an attempt at centralizing the set of subprogram options (and also
adding better error messages). My intent was to make it difficult to miss
updating all the relevant places when adding a new subprogram, but I'll
admit the patch is a bit more complicated than I was hoping.
--
nathan
>From 5b2a98ad0571e93d789bec4bb9343f1307fc3f53 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@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)
