-O switch
postgres --help:
-o OPTIONS pass "OPTIONS" to each server process (obsolete)
This was marked obsolete in 2006 (86c23a6eb28).
Is it perhaps time to get rid of it?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
postgres --help:
-o OPTIONS pass "OPTIONS" to each server process (obsolete)
This was marked obsolete in 2006 (86c23a6eb28).
I don't think it's really obsolete ... don't we use that to pass
PGOPTIONS through from the client?
regards, tom lane
On Thu, Oct 29, 2020 at 4:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
postgres --help:
-o OPTIONS pass "OPTIONS" to each server process (obsolete)This was marked obsolete in 2006 (86c23a6eb28).
I don't think it's really obsolete ... don't we use that to pass
PGOPTIONS through from the client?
Then it probably shouldn't be labeled as obsolete :)
That said, I don't think we do, or I'm misunderstanding what you mean.
The startup packet which holds the client options is not read until
we're already in the child process, so there is no further exec to be
done?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Oct 29, 2020 at 4:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't think it's really obsolete ... don't we use that to pass
PGOPTIONS through from the client?
That said, I don't think we do, or I'm misunderstanding what you mean.
The startup packet which holds the client options is not read until
we're already in the child process, so there is no further exec to be
done?
[ pokes around... ] Ah, you're right, that stuff goes through
port->cmdline_options now. It looks like the mechanism for -o
is the postmaster's ExtraOptions variable, which we could get
rid of this way. Seems like a reasonable thing, especially since
we unified all the other postmaster/postgres options already.
regards, tom lane
On Thu, Oct 29, 2020 at 5:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Thu, Oct 29, 2020 at 4:45 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't think it's really obsolete ... don't we use that to pass
PGOPTIONS through from the client?That said, I don't think we do, or I'm misunderstanding what you mean.
The startup packet which holds the client options is not read until
we're already in the child process, so there is no further exec to be
done?[ pokes around... ] Ah, you're right, that stuff goes through
port->cmdline_options now. It looks like the mechanism for -o
is the postmaster's ExtraOptions variable, which we could get
rid of this way. Seems like a reasonable thing, especially since
we unified all the other postmaster/postgres options already.
PFA a patch to do this.
Initially I kept the dynamic argv/argc in even though it's now
hardcoded, in case we wanted to add something back. But given the way
it looks now, perhaps we should just get rid of BackendRun()
completely and directly call PostgresMain()? Or keep BackendRun() with
just setting the TopMemoryContext, but removing the dynamic parts?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Attachments:
remove_option_o.patchtext/x-patch; charset=US-ASCII; name=remove_option_o.patchDownload
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
index fda678e345..4aaa7abe1a 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -280,32 +280,6 @@ PostgreSQL documentation
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>-o <replaceable class="parameter">extra-options</replaceable></option></term>
- <listitem>
- <para>
- The command-line-style arguments specified in <replaceable
- class="parameter">extra-options</replaceable> are passed to
- all server processes started by this
- <command>postgres</command> process.
- </para>
-
- <para>
- Spaces within <replaceable class="parameter">extra-options</replaceable> are
- considered to separate arguments, unless escaped with a backslash
- (<literal>\</literal>); write <literal>\\</literal> to represent a literal
- backslash. Multiple arguments can also be specified via multiple
- uses of <option>-o</option>.
- </para>
-
- <para>
- The use of this option is obsolete; all command-line options
- for server processes can be specified directly on the
- <command>postgres</command> command line.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><option>-p <replaceable class="parameter">port</replaceable></option></term>
<listitem>
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index a4dd233c7f..b6e5128832 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -323,7 +323,6 @@ help(const char *progname)
printf(_(" -l enable SSL connections\n"));
#endif
printf(_(" -N MAX-CONNECT maximum number of allowed connections\n"));
- printf(_(" -o OPTIONS pass \"OPTIONS\" to each server process (obsolete)\n"));
printf(_(" -p PORT port number to listen on\n"));
printf(_(" -s show statistics after each query\n"));
printf(_(" -S WORK-MEM set amount of memory for sorts (in kB)\n"));
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 959e3b8873..fcfe6fbc84 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -105,7 +105,6 @@
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "libpq/pqsignal.h"
-#include "miscadmin.h"
#include "pg_getopt.h"
#include "pgstat.h"
#include "port/pg_bswap.h"
@@ -219,12 +218,6 @@ int ReservedBackends;
/* The socket(s) we're listening to. */
#define MAXLISTEN 64
static pgsocket ListenSocket[MAXLISTEN];
-
-/*
- * Set by the -o option
- */
-static char ExtraOptions[MAXPGPATH];
-
/*
* These globals control the behavior of the postmaster in case some
* backend dumps core. Normally, it kills all peers of the dead backend
@@ -537,7 +530,6 @@ typedef struct
#endif
char my_exec_path[MAXPGPATH];
char pkglib_path[MAXPGPATH];
- char ExtraOptions[MAXPGPATH];
} BackendParameters;
static void read_backend_variables(char *id, Port *port);
@@ -773,13 +765,6 @@ PostmasterMain(int argc, char *argv[])
SetConfigOption("allow_system_table_mods", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
- case 'o':
- /* Other options to pass to the backend on the command line */
- snprintf(ExtraOptions + strlen(ExtraOptions),
- sizeof(ExtraOptions) - strlen(ExtraOptions),
- " %s", optarg);
- break;
-
case 'P':
SetConfigOption("ignore_system_indexes", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
@@ -4496,26 +4481,14 @@ BackendRun(Port *port)
/*
* Now, build the argv vector that will be given to PostgresMain.
- *
- * The maximum possible number of commandline arguments that could come
- * from ExtraOptions is (strlen(ExtraOptions) + 1) / 2; see
- * pg_split_opts().
*/
maxac = 2; /* for fixed args supplied below */
- maxac += (strlen(ExtraOptions) + 1) / 2;
av = (char **) MemoryContextAlloc(TopMemoryContext,
maxac * sizeof(char *));
ac = 0;
av[ac++] = "postgres";
-
- /*
- * Pass any backend switches specified with -o on the postmaster's own
- * command line. We assume these are secure.
- */
- pg_split_opts(av, &ac, ExtraOptions);
-
av[ac] = NULL;
Assert(ac < maxac);
@@ -6253,8 +6226,6 @@ save_backend_variables(BackendParameters *param, Port *port,
strlcpy(param->pkglib_path, pkglib_path, MAXPGPATH);
- strlcpy(param->ExtraOptions, ExtraOptions, MAXPGPATH);
-
return true;
}
@@ -6485,8 +6456,6 @@ restore_backend_variables(BackendParameters *param, Port *port)
strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH);
- strlcpy(ExtraOptions, param->ExtraOptions, MAXPGPATH);
-
/*
* We need to restore fd.c's counts of externally-opened FDs; to avoid
* confusion, be sure to do this after restoring max_safe_fds. (Note:
Magnus Hagander <magnus@hagander.net> writes:
PFA a patch to do this.
One thing you missed is that the getopt() calls in both postmaster.c
and postgres.c have 'o:' entries that should be removed. Also IIRC
there is a "case 'o'" in postgres.c to go along with that.
Initially I kept the dynamic argv/argc in even though it's now
hardcoded, in case we wanted to add something back. But given the way
it looks now, perhaps we should just get rid of BackendRun()
completely and directly call PostgresMain()? Or keep BackendRun() with
just setting the TopMemoryContext, but removing the dynamic parts?
I'd be inclined to keep it as-is for now. It's not adding any significant
amount of cycles compared to the process fork, so we might as well
preserve flexibility.
Is it really possible to not include miscadmin.h in postmaster.c?
I find that a bit surprising.
regards, tom lane
On Mon, Nov 2, 2020 at 6:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
PFA a patch to do this.
One thing you missed is that the getopt() calls in both postmaster.c
and postgres.c have 'o:' entries that should be removed. Also IIRC
there is a "case 'o'" in postgres.c to go along with that.
Ha. Of course. Oops.
PFA updated.
Initially I kept the dynamic argv/argc in even though it's now
hardcoded, in case we wanted to add something back. But given the way
it looks now, perhaps we should just get rid of BackendRun()
completely and directly call PostgresMain()? Or keep BackendRun() with
just setting the TopMemoryContext, but removing the dynamic parts?I'd be inclined to keep it as-is for now. It's not adding any significant
amount of cycles compared to the process fork, so we might as well
preserve flexibility.Is it really possible to not include miscadmin.h in postmaster.c?
I find that a bit surprising.
I did too, but having removed it postmaster.c still compiles fine
without warnings for me. It did also pass the cfbot build step, but it
might be that it'll eventually break down on some more different
buildfarm animal.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Attachments:
remove_option_o_2.patchtext/x-patch; charset=US-ASCII; name=remove_option_o_2.patchDownload
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
index fda678e345..4aaa7abe1a 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -280,32 +280,6 @@ PostgreSQL documentation
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>-o <replaceable class="parameter">extra-options</replaceable></option></term>
- <listitem>
- <para>
- The command-line-style arguments specified in <replaceable
- class="parameter">extra-options</replaceable> are passed to
- all server processes started by this
- <command>postgres</command> process.
- </para>
-
- <para>
- Spaces within <replaceable class="parameter">extra-options</replaceable> are
- considered to separate arguments, unless escaped with a backslash
- (<literal>\</literal>); write <literal>\\</literal> to represent a literal
- backslash. Multiple arguments can also be specified via multiple
- uses of <option>-o</option>.
- </para>
-
- <para>
- The use of this option is obsolete; all command-line options
- for server processes can be specified directly on the
- <command>postgres</command> command line.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><option>-p <replaceable class="parameter">port</replaceable></option></term>
<listitem>
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index a4dd233c7f..b6e5128832 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -323,7 +323,6 @@ help(const char *progname)
printf(_(" -l enable SSL connections\n"));
#endif
printf(_(" -N MAX-CONNECT maximum number of allowed connections\n"));
- printf(_(" -o OPTIONS pass \"OPTIONS\" to each server process (obsolete)\n"));
printf(_(" -p PORT port number to listen on\n"));
printf(_(" -s show statistics after each query\n"));
printf(_(" -S WORK-MEM set amount of memory for sorts (in kB)\n"));
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 959e3b8873..5abccf9e07 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -105,7 +105,6 @@
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "libpq/pqsignal.h"
-#include "miscadmin.h"
#include "pg_getopt.h"
#include "pgstat.h"
#include "port/pg_bswap.h"
@@ -219,12 +218,6 @@ int ReservedBackends;
/* The socket(s) we're listening to. */
#define MAXLISTEN 64
static pgsocket ListenSocket[MAXLISTEN];
-
-/*
- * Set by the -o option
- */
-static char ExtraOptions[MAXPGPATH];
-
/*
* These globals control the behavior of the postmaster in case some
* backend dumps core. Normally, it kills all peers of the dead backend
@@ -537,7 +530,6 @@ typedef struct
#endif
char my_exec_path[MAXPGPATH];
char pkglib_path[MAXPGPATH];
- char ExtraOptions[MAXPGPATH];
} BackendParameters;
static void read_backend_variables(char *id, Port *port);
@@ -694,7 +686,7 @@ PostmasterMain(int argc, char *argv[])
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
*/
- while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
+ while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:W:-:")) != -1)
{
switch (opt)
{
@@ -773,13 +765,6 @@ PostmasterMain(int argc, char *argv[])
SetConfigOption("allow_system_table_mods", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
- case 'o':
- /* Other options to pass to the backend on the command line */
- snprintf(ExtraOptions + strlen(ExtraOptions),
- sizeof(ExtraOptions) - strlen(ExtraOptions),
- " %s", optarg);
- break;
-
case 'P':
SetConfigOption("ignore_system_indexes", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
@@ -4496,26 +4481,14 @@ BackendRun(Port *port)
/*
* Now, build the argv vector that will be given to PostgresMain.
- *
- * The maximum possible number of commandline arguments that could come
- * from ExtraOptions is (strlen(ExtraOptions) + 1) / 2; see
- * pg_split_opts().
*/
maxac = 2; /* for fixed args supplied below */
- maxac += (strlen(ExtraOptions) + 1) / 2;
av = (char **) MemoryContextAlloc(TopMemoryContext,
maxac * sizeof(char *));
ac = 0;
av[ac++] = "postgres";
-
- /*
- * Pass any backend switches specified with -o on the postmaster's own
- * command line. We assume these are secure.
- */
- pg_split_opts(av, &ac, ExtraOptions);
-
av[ac] = NULL;
Assert(ac < maxac);
@@ -6253,8 +6226,6 @@ save_backend_variables(BackendParameters *param, Port *port,
strlcpy(param->pkglib_path, pkglib_path, MAXPGPATH);
- strlcpy(param->ExtraOptions, ExtraOptions, MAXPGPATH);
-
return true;
}
@@ -6485,8 +6456,6 @@ restore_backend_variables(BackendParameters *param, Port *port)
strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH);
- strlcpy(ExtraOptions, param->ExtraOptions, MAXPGPATH);
-
/*
* We need to restore fd.c's counts of externally-opened FDs; to avoid
* confusion, be sure to do this after restoring max_safe_fds. (Note:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 411cfadbff..7c5f7c775b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3554,7 +3554,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
* postmaster/postmaster.c (the option sets should not conflict) and with
* the common help() function in main/main.c.
*/
- while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
+ while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:v:W:-:")) != -1)
{
switch (flag)
{
@@ -3632,10 +3632,6 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
SetConfigOption("allow_system_table_mods", "true", ctx, gucsource);
break;
- case 'o':
- errs++;
- break;
-
case 'P':
SetConfigOption("ignore_system_indexes", "true", ctx, gucsource);
break;
Magnus Hagander <magnus@hagander.net> writes:
[ remove_option_o_2.patch ]
This seems committable to me now, although ...
On Mon, Nov 2, 2020 at 6:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
Initially I kept the dynamic argv/argc in even though it's now
hardcoded, in case we wanted to add something back. But given the way
it looks now, perhaps we should just get rid of BackendRun()
completely and directly call PostgresMain()? Or keep BackendRun() with
just setting the TopMemoryContext, but removing the dynamic parts?
I'd be inclined to keep it as-is for now. It's not adding any significant
amount of cycles compared to the process fork, so we might as well
preserve flexibility.
... looking at this again, BackendRun certainly looks ridiculously
over-engineered for what it still does. If we keep it like this, we
should at least add a comment along the lines of "We once had the
ability to pass additional arguments to PostgresMain, and may someday
want to do that again". But I wouldn't object to getting rid of the
dynamic construction of the arg array, and the debug output too.
regards, tom lane
On Wed, Nov 4, 2020 at 2:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
[ remove_option_o_2.patch ]
This seems committable to me now, although ...
On Mon, Nov 2, 2020 at 6:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
Initially I kept the dynamic argv/argc in even though it's now
hardcoded, in case we wanted to add something back. But given the way
it looks now, perhaps we should just get rid of BackendRun()
completely and directly call PostgresMain()? Or keep BackendRun() with
just setting the TopMemoryContext, but removing the dynamic parts?I'd be inclined to keep it as-is for now. It's not adding any significant
amount of cycles compared to the process fork, so we might as well
preserve flexibility.... looking at this again, BackendRun certainly looks ridiculously
over-engineered for what it still does. If we keep it like this, we
should at least add a comment along the lines of "We once had the
ability to pass additional arguments to PostgresMain, and may someday
want to do that again". But I wouldn't object to getting rid of the
dynamic construction of the arg array, and the debug output too.
Yeah, looking at it again, I agree. PFA an updated patch, which I'll
go ahead and push shortly.
I do noticed when looking through this -- the comment before the function says:
* returns:
* Shouldn't return at all.
* If PostgresMain() fails, return status.
I'm pretty sure that's incorrect in the current branches as well,
since it's a void function it will never return anything. Pretty sure
it should just have the first point and not the second one there, or
is this trying to convey some meaning I'm just not getting?
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
Attachments:
remove_option_o_3.patchtext/x-patch; charset=US-ASCII; name=remove_option_o_3.patchDownload
diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
index fda678e345..4aaa7abe1a 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -280,32 +280,6 @@ PostgreSQL documentation
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>-o <replaceable class="parameter">extra-options</replaceable></option></term>
- <listitem>
- <para>
- The command-line-style arguments specified in <replaceable
- class="parameter">extra-options</replaceable> are passed to
- all server processes started by this
- <command>postgres</command> process.
- </para>
-
- <para>
- Spaces within <replaceable class="parameter">extra-options</replaceable> are
- considered to separate arguments, unless escaped with a backslash
- (<literal>\</literal>); write <literal>\\</literal> to represent a literal
- backslash. Multiple arguments can also be specified via multiple
- uses of <option>-o</option>.
- </para>
-
- <para>
- The use of this option is obsolete; all command-line options
- for server processes can be specified directly on the
- <command>postgres</command> command line.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term><option>-p <replaceable class="parameter">port</replaceable></option></term>
<listitem>
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index a4dd233c7f..b6e5128832 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -323,7 +323,6 @@ help(const char *progname)
printf(_(" -l enable SSL connections\n"));
#endif
printf(_(" -N MAX-CONNECT maximum number of allowed connections\n"));
- printf(_(" -o OPTIONS pass \"OPTIONS\" to each server process (obsolete)\n"));
printf(_(" -p PORT port number to listen on\n"));
printf(_(" -s show statistics after each query\n"));
printf(_(" -S WORK-MEM set amount of memory for sorts (in kB)\n"));
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 959e3b8873..f6d9b0fa8b 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -105,7 +105,6 @@
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "libpq/pqsignal.h"
-#include "miscadmin.h"
#include "pg_getopt.h"
#include "pgstat.h"
#include "port/pg_bswap.h"
@@ -219,12 +218,6 @@ int ReservedBackends;
/* The socket(s) we're listening to. */
#define MAXLISTEN 64
static pgsocket ListenSocket[MAXLISTEN];
-
-/*
- * Set by the -o option
- */
-static char ExtraOptions[MAXPGPATH];
-
/*
* These globals control the behavior of the postmaster in case some
* backend dumps core. Normally, it kills all peers of the dead backend
@@ -537,7 +530,6 @@ typedef struct
#endif
char my_exec_path[MAXPGPATH];
char pkglib_path[MAXPGPATH];
- char ExtraOptions[MAXPGPATH];
} BackendParameters;
static void read_backend_variables(char *id, Port *port);
@@ -694,7 +686,7 @@ PostmasterMain(int argc, char *argv[])
* tcop/postgres.c (the option sets should not conflict) and with the
* common help() function in main/main.c.
*/
- while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1)
+ while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:W:-:")) != -1)
{
switch (opt)
{
@@ -773,13 +765,6 @@ PostmasterMain(int argc, char *argv[])
SetConfigOption("allow_system_table_mods", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
- case 'o':
- /* Other options to pass to the backend on the command line */
- snprintf(ExtraOptions + strlen(ExtraOptions),
- sizeof(ExtraOptions) - strlen(ExtraOptions),
- " %s", optarg);
- break;
-
case 'P':
SetConfigOption("ignore_system_indexes", "true", PGC_POSTMASTER, PGC_S_ARGV);
break;
@@ -4489,48 +4474,11 @@ BackendInitialize(Port *port)
static void
BackendRun(Port *port)
{
- char **av;
- int maxac;
- int ac;
- int i;
+ char *av[2];
+ const int ac = 1;
- /*
- * Now, build the argv vector that will be given to PostgresMain.
- *
- * The maximum possible number of commandline arguments that could come
- * from ExtraOptions is (strlen(ExtraOptions) + 1) / 2; see
- * pg_split_opts().
- */
- maxac = 2; /* for fixed args supplied below */
- maxac += (strlen(ExtraOptions) + 1) / 2;
-
- av = (char **) MemoryContextAlloc(TopMemoryContext,
- maxac * sizeof(char *));
- ac = 0;
-
- av[ac++] = "postgres";
-
- /*
- * Pass any backend switches specified with -o on the postmaster's own
- * command line. We assume these are secure.
- */
- pg_split_opts(av, &ac, ExtraOptions);
-
- av[ac] = NULL;
-
- Assert(ac < maxac);
-
- /*
- * Debug: print arguments being passed to backend
- */
- ereport(DEBUG3,
- (errmsg_internal("%s child[%d]: starting with (",
- progname, (int) getpid())));
- for (i = 0; i < ac; ++i)
- ereport(DEBUG3,
- (errmsg_internal("\t%s", av[i])));
- ereport(DEBUG3,
- (errmsg_internal(")")));
+ av[0] = "postgres";
+ av[1] = NULL;
/*
* Make sure we aren't in PostmasterContext anymore. (We can't delete it
@@ -6253,8 +6201,6 @@ save_backend_variables(BackendParameters *param, Port *port,
strlcpy(param->pkglib_path, pkglib_path, MAXPGPATH);
- strlcpy(param->ExtraOptions, ExtraOptions, MAXPGPATH);
-
return true;
}
@@ -6485,8 +6431,6 @@ restore_backend_variables(BackendParameters *param, Port *port)
strlcpy(pkglib_path, param->pkglib_path, MAXPGPATH);
- strlcpy(ExtraOptions, param->ExtraOptions, MAXPGPATH);
-
/*
* We need to restore fd.c's counts of externally-opened FDs; to avoid
* confusion, be sure to do this after restoring max_safe_fds. (Note:
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 411cfadbff..7c5f7c775b 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -3554,7 +3554,7 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
* postmaster/postmaster.c (the option sets should not conflict) and with
* the common help() function in main/main.c.
*/
- while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:v:W:-:")) != -1)
+ while ((flag = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOPp:r:S:sTt:v:W:-:")) != -1)
{
switch (flag)
{
@@ -3632,10 +3632,6 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx,
SetConfigOption("allow_system_table_mods", "true", ctx, gucsource);
break;
- case 'o':
- errs++;
- break;
-
case 'P':
SetConfigOption("ignore_system_indexes", "true", ctx, gucsource);
break;
Magnus Hagander <magnus@hagander.net> writes:
On Wed, Nov 4, 2020 at 2:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
... looking at this again, BackendRun certainly looks ridiculously
over-engineered for what it still does.
Yeah, looking at it again, I agree. PFA an updated patch, which I'll
go ahead and push shortly.
LGTM.
I do noticed when looking through this -- the comment before the function says:
* returns:
* Shouldn't return at all.
* If PostgresMain() fails, return status.
I'm pretty sure that's incorrect in the current branches as well,
since it's a void function it will never return anything. Pretty sure
it should just have the first point and not the second one there, or
is this trying to convey some meaning I'm just not getting?
Looking at old versions, BackendRun and PostgresMain used to be
declared to return int. Whoever changed that to void evidently
missed updating this comment.
I'd reduce the whole thing to "Doesn't return." If you were feeling
really ambitious you could start plastering pg_attribute_noreturn() on
these functions ... but since that would be placed on the declarations,
a comment here would still be in order probably.
regards, tom lane
On Mon, Nov 9, 2020 at 4:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Magnus Hagander <magnus@hagander.net> writes:
On Wed, Nov 4, 2020 at 2:10 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
... looking at this again, BackendRun certainly looks ridiculously
over-engineered for what it still does.Yeah, looking at it again, I agree. PFA an updated patch, which I'll
go ahead and push shortly.LGTM.
Pushed.
I do noticed when looking through this -- the comment before the function says:
* returns:
* Shouldn't return at all.
* If PostgresMain() fails, return status.I'm pretty sure that's incorrect in the current branches as well,
since it's a void function it will never return anything. Pretty sure
it should just have the first point and not the second one there, or
is this trying to convey some meaning I'm just not getting?Looking at old versions, BackendRun and PostgresMain used to be
declared to return int. Whoever changed that to void evidently
missed updating this comment.I'd reduce the whole thing to "Doesn't return." If you were feeling
really ambitious you could start plastering pg_attribute_noreturn() on
these functions ... but since that would be placed on the declarations,
a comment here would still be in order probably.
They're already marked pg_attribute_noreturn() in the declarations.
It's just the comment that was a bit out of date.
I'll go fix that one.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/