Thread: fixing pg_ctl with relative paths
There have been some complaints[1][2] in the past about pg_ctl not playing nice with relative path specifications for the datadir. Here's a concise illustration: $ mkdir /tmp/mydata/ && initdb /tmp/mydata/ $ cd /tmp/ $ pg_ctl -D ./mydata/ start $ cd / $ pg_ctl -D /tmp/mydata/ restart IMO it's pretty hard to defend the behavior of the last step, where pg_ctl knows exactly which datadir the user has specified, and succeeds in stopping the server but not starting it. Digging into this gripe, a related problem I noticed is that `pg_ctl ... restart` doesn't always preserve the "-D $DATADIR" argument as the following comment suggests it should[4]: * We could pass PGDATA just in an environment * variable but we do -D too for clearer postmaster * 'ps' display Specifically, if one passes in additional -o options, e.g. $ pg_ctl -D /tmp/mydata/ -o "-N 10" restart after which postmaster.opts will be missing the "-D ..." argument which is otherwise recorded, and the `ps` output is similarly abridged. Anyway, Tom suggested[3] two possible ways of fixing the original gripe, and I went with his latter suggestion, | for pg_ctl restart to override that | option with its freshly-derived idea of where the data directory is mainly so we don't need to worry about the impact of changing the appearance of postmaster.opts, plus it seems like this code fits better in pg_ctl.c rather than the postmaster (where the postmaster.opts file is actually written). The fix seems to be pretty simple, namely stripping post_opts of the old "-D ... " portion and having the new specification, if any, be used instead. I believe the attached patch should fix these two infelicities. Full disclosure: the strip_datadirs() function makes no attempt to properly handle pathnames containing quotes. There seems to be some, uh, confusion in the existing code about how these should be handled already. For instance, $ mkdir "/tmp/here's a \" quote" $ initdb "/tmp/here's a \" quote" How to successfully start, restart, and stop the server with pg_ctl is left as an exercise for the reader. So I tried to avoid that can of worms with this patch, though I'm happy to robustify strip_datadirs() if we'd like to properly support such pathnames, and there's consensus on how to standardize the escaping. Josh [1] http://www.postgresql.org/message-id/CAA-aLv72O+NegjAipHORmbqSMZTkZayaTxrd+C9v60YbmMmZUQ@mail.gmail.com [2] http://www.postgresql.org/message-id/CAK3UJRGABxWSOCXnAsSYw5BfR4D9ageXF+6GtsRVm-LtfWfW=g@mail.gmail.com [3] http://www.postgresql.org/message-id/27233.1350234453@sss.pgh.pa.us [4] Note, ps output and postmaster.opts will not include the datadir specification if you rely solely on the PGDATA environment variable for pg_ctl
Attachment
On January 23, 2013 9:13 AM Josh Kupershmidt wrote: >There have been some complaints[1][2] in the past about pg_ctl not playing nice with relative path specifications for the datadir. Here's a concise illustration: > > $ mkdir /tmp/mydata/ && initdb /tmp/mydata/ > $ cd /tmp/ > $ pg_ctl -D ./mydata/ start > $ cd / > $ pg_ctl -D /tmp/mydata/ restart > >IMO it's pretty hard to defend the behavior of the last step, where pg_ctl knows exactly which datadir the user has specified, and succeeds in stopping the >server but not starting it. > >Digging into this gripe, a related problem I noticed is that `pg_ctl ... restart` doesn't always preserve the "-D $DATADIR" argument as the following comment >suggests it should[4]: > > * We could pass PGDATA just in an environment > * variable but we do -D too for clearer postmaster > * 'ps' display > >Specifically, if one passes in additional -o options, e.g. > > $ pg_ctl -D /tmp/mydata/ -o "-N 10" restart > >after which postmaster.opts will be missing the "-D ..." argument which is otherwise recorded, and the `ps` output is similarly abridged. > >Anyway, Tom suggested[3] two possible ways of fixing the original gripe, and I went with his latter suggestion, > >| for pg_ctl restart to override that >| option with its freshly-derived idea of where the data directory is > >mainly so we don't need to worry about the impact of changing the appearance of postmaster.opts, plus it seems like this code fits better in pg_ctl.c rather than >the postmaster (where the postmaster.opts file is actually written). The fix seems to be pretty simple, namely stripping post_opts of the old "-D ... " portion >and having the new specification, if any, be used instead. I believe the attached patch should fix these two infelicities. > >Full disclosure: the strip_datadirs() function makes no attempt to properly handle pathnames containing quotes. There seems to be some, uh, confusion in the >existing code about how these should be handled already. For instance, > > $ mkdir "/tmp/here's a \" quote" > $ initdb "/tmp/here's a \" quote" > >How to successfully start, restart, and stop the server with pg_ctl is left as an exercise for the reader. So I tried to avoid that can of worms with this patch, >though I'm happy to robustify strip_datadirs() if we'd like to properly support such pathnames, and there's consensus on how to standardize the escaping. > >[1] http://www.postgresql.org/message-id/CAA-aLv72O+NegjAipHORmbqSMZTkZayaTxrd+C 9v60YbmMmZUQ@mail.gmail.com >[2] http://www.postgresql.org/message-id/CAK3UJRGABxWSOCXnAsSYw5BfR4D9ageXF+6Gts RVm-LtfWfW=g@mail.gmail.com >[3] http://www.postgresql.org/message-id/27233.1350234453@sss.pgh.pa.us >[4] Note, ps output and postmaster.opts will not include the datadir specification if you rely solely on the PGDATA environment variable for pg_ctl Please find the review of the patch. Basic stuff: ------------ - Patch applies OK - Compiles cleanly with no warnings - Regression tests pass. What it does: ------------- The restart functionality of pg_ctl has problems with relative paths. This patch removes the problems arising during restart. This patch strips the data directory which is present in the postmaster options and keep the rest of the options already provided incase if user not provided any options during restart. Code Review: ------------ +if (orig_post_opts) { + post_opts = strip_datadirs(orig_post_opts); +} No need of "{}" as the only one statement block is present in the if block. + free(tmp); The above statement can be moved inside the if (*(trailing_quote + 1) != '\0') { where it's exact usage is present. Testing: -------- I tested this feature with different postmaster options and database folder names, found no problem. The database folder with quotes present in it is any way having problems with pg_ctl. I feel the strip_datadirs() function header explanation is providing good understanding. Overall the patch is good. It makes the pg_ctl restart functionality works well. Regards, Hari babu
On Tue, Jun 25, 2013 at 2:28 AM, Hari Babu <haribabu.kommi@huawei.com> wrote: > Please find the review of the patch. Thank you for reviewing! > Code Review: > ------------ > +if (orig_post_opts) { > + post_opts = strip_datadirs(orig_post_opts); > +} > > No need of "{}" as the only one statement block is present in the if block. OK. > + free(tmp); > > The above statement can be moved inside the if (*(trailing_quote + 1) != > '\0') { > where it's exact usage is present. Right. > Testing: > -------- > I tested this feature with different postmaster options and database folder > names, found no problem. > > > The database folder with quotes present in it is any way having problems > with pg_ctl. > I feel the strip_datadirs() function header explanation is providing good > understanding. > Overall the patch is good. It makes the pg_ctl restart functionality works > well. Thanks for the feedback. Attached is a rebased version of the patch with the two small issues noted fixed. Josh
Attachment
On June 26, 2013 5:02 AM Josh Kupershmidt wrote: >Thanks for the feedback. Attached is a rebased version of the patch with the two small issues noted fixed. Patch is good, I marked the patch as ready for committer. Regards, Hari babu
On Wed, Jun 26, 2013 at 2:36 PM, Hari Babu <haribabu.kommi@huawei.com> wrote: > On June 26, 2013 5:02 AM Josh Kupershmidt wrote: >>Thanks for the feedback. Attached is a rebased version of the patch with > the two small issues noted fixed. The following description in the document of pg_ctl needs to be modified? restart might fail if relative paths specified were specified on the command-line during server start. +#define DATADIR_SPEC "\"-D\" \"" + + datadir = strstr(post_opts, DATADIR_SPEC); Though this is a corner case, the patch doesn't seem to handle properly the case where "-D" appears as other option value, e.g., -k option value, in postmaster.opts file. Just idea to work around that problem is to just append the specified -D option and value to post_opts. IOW, -D option and value appear twice in post_opts. In this case, posteriorly-located ones are used in the end. Thought? Regards, -- Fujii Masao
On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Wed, Jun 26, 2013 at 2:36 PM, Hari Babu <haribabu.kommi@huawei.com> wrote: >> On June 26, 2013 5:02 AM Josh Kupershmidt wrote: >>>Thanks for the feedback. Attached is a rebased version of the patch with >> the two small issues noted fixed. > > The following description in the document of pg_ctl needs to be modified? > > restart might fail if relative paths specified were specified on > the command-line during server start. Right, that caveat could go away. > +#define DATADIR_SPEC "\"-D\" \"" > + > + datadir = strstr(post_opts, DATADIR_SPEC); > > Though this is a corner case, the patch doesn't seem to handle properly the case > where "-D" appears as other option value, e.g., -k option value, in > postmaster.opts > file. Could I see a command-line example of what you mean? > Just idea to work around that problem is to just append the specified -D option > and value to post_opts. IOW, -D option and value appear twice in post_opts. > In this case, posteriorly-located ones are used in the end. Thought? Hrm, I think we'd have to be careful that postmaster.opts doesn't accumulate an additional -D specification with every restart. Josh
On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >> On Wed, Jun 26, 2013 at 2:36 PM, Hari Babu <haribabu.kommi@huawei.com> wrote: >>> On June 26, 2013 5:02 AM Josh Kupershmidt wrote: >>>>Thanks for the feedback. Attached is a rebased version of the patch with >>> the two small issues noted fixed. >> >> The following description in the document of pg_ctl needs to be modified? >> >> restart might fail if relative paths specified were specified on >> the command-line during server start. > > Right, that caveat could go away. > >> +#define DATADIR_SPEC "\"-D\" \"" >> + >> + datadir = strstr(post_opts, DATADIR_SPEC); >> >> Though this is a corner case, the patch doesn't seem to handle properly the case >> where "-D" appears as other option value, e.g., -k option value, in >> postmaster.opts >> file. > > Could I see a command-line example of what you mean? postmaster -k "-D", for example. Of course, it's really a corner case :) Another corner case is, for example, pg_ctl -D test1 -o "-D test2", .... that is, multiple -D specifications appear in the command-line. Can we overlook these cases? >> Just idea to work around that problem is to just append the specified -D option >> and value to post_opts. IOW, -D option and value appear twice in post_opts. >> In this case, posteriorly-located ones are used in the end. Thought? > > Hrm, I think we'd have to be careful that postmaster.opts doesn't > accumulate an additional -D specification with every restart. Yes. Oh, I was thinking that postmaster writes only -D specification which postmaster actually uses, in the opts file. So that accumulation would not happen, I thought. But that's not true. Postmaster writes all the specified arguments in the opts file. Regards, -- Fujii Masao
On Fri, Jun 28, 2013 at 12:47 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > > Another corner case is, for example, pg_ctl -D test1 -o "-D test2", .... > that is, multiple -D specifications appear in the command-line. The patch handles this case properly. Sorry for noise.. Regards, -- Fujii Masao
On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >>> Though this is a corner case, the patch doesn't seem to handle properly the case >>> where "-D" appears as other option value, e.g., -k option value, in >>> postmaster.opts >>> file. >> >> Could I see a command-line example of what you mean? > > postmaster -k "-D", for example. Of course, it's really a corner case :) Oh, I see. I was able to trip up strip_datadirs() with something like $ PGDATA="/my/data/" postmaster -k "-D" -S 100 & $ pg_ctl -D /my/data/ restart that example causes pg_ctl to fail to start the server after stopping it, although perhaps you could even trick the server into starting with the wrong options. Of course, similar problems exists today in other cases, such as with the relative paths issue this patch is trying to address, or a datadir containing embedded quotes. I am eager to see the relative paths issue fixed, but maybe we need to bite the bullet and sort out the escaping of command-line options in the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \" quote" can consistently be used by pg_ctl {start|stop|restart} before we can fix this wart. Josh
On Mon, Jul 1, 2013 at 08:10:14PM -0400, Josh Kupershmidt wrote: > On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > >>> Though this is a corner case, the patch doesn't seem to handle properly the case > >>> where "-D" appears as other option value, e.g., -k option value, in > >>> postmaster.opts > >>> file. > >> > >> Could I see a command-line example of what you mean? > > > > postmaster -k "-D", for example. Of course, it's really a corner case :) > > Oh, I see. I was able to trip up strip_datadirs() with something like > > $ PGDATA="/my/data/" postmaster -k "-D" -S 100 & > $ pg_ctl -D /my/data/ restart > > that example causes pg_ctl to fail to start the server after stopping > it, although perhaps you could even trick the server into starting > with the wrong options. Of course, similar problems exists today in > other cases, such as with the relative paths issue this patch is > trying to address, or a datadir containing embedded quotes. > > I am eager to see the relative paths issue fixed, but maybe we need to > bite the bullet and sort out the escaping of command-line options in > the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \" > quote" can consistently be used by pg_ctl {start|stop|restart} before > we can fix this wart. Where are we on this patch? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
At 2014-01-28 21:11:54, "Bruce Momjian" <bruce@momjian.us> wrote:
>On Mon, Jul 1, 2013 at 08:10:14PM -0400, Josh Kupershmidt wrote: >> On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >> > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote: >> >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >> >>> Though this is a corner case, the patch doesn't seem to handle properly the case >> >>> where "-D" appears as other option value, e.g., -k option value, in >> >>> postmaster.opts >> >>> file. >> >> >> >> Could I see a command-line example of what you mean? >> > >> > postmaster -k "-D", for example. Of course, it's really a corner case :) >> >> Oh, I see. I was able to trip up strip_datadirs() with something like >> >> $ PGDATA="/my/data/" postmaster -k "-D" -S 100 & >> $ pg_ctl -D /my/data/ restart >> >> that example causes pg_ctl to fail to start the server after stopping >> it, although perhaps you could even trick the server into starting >> with the wrong options. Of course, similar problems exists today in >> other cases, such as with the relative paths issue this patch is >> trying to address, or a datadir containing embedded quotes. >> >> I am eager to see the relative paths issue fixed, but maybe we need to >> bite the bullet and sort out the escaping of command-line options in >> the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \" >> quote" can consistently be used by pg_ctl {start|stop|restart} before >> we can fix this wart. > >Where are we on this patch? > >-- > Bruce Momjian <bruce@momjian.us> http://momjian.us > EnterpriseDB http://enterprisedb.com > > + Everyone has their own god. + > > >-- >Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >To make changes to your subscription: >http://www.postgresql.org/mailpref/pgsql-hackersHi, I encountered the same problem.I want to know is there a final conclusion?thank you very much!
At Fri, 31 Jul 2020 09:42:42 +0800 (CST), ZHAOWANCHENG <zhaowcheng@163.com> wrote in > At 2014-01-28 21:11:54, "Bruce Momjian" <bruce@momjian.us> wrote: > >On Mon, Jul 1, 2013 at 08:10:14PM -0400, Josh Kupershmidt wrote: > >> On Thu, Jun 27, 2013 at 11:47 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > >> > On Thu, Jun 27, 2013 at 10:36 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote: > >> >> On Wed, Jun 26, 2013 at 12:22 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > >> >>> Though this is a corner case, the patch doesn't seem to handle properly the case > >> >>> where "-D" appears as other option value, e.g., -k option value, in > >> >>> postmaster.opts > >> >>> file. > >> >> > >> >> Could I see a command-line example of what you mean? > >> > > >> > postmaster -k "-D", for example. Of course, it's really a corner case :) > >> > >> Oh, I see. I was able to trip up strip_datadirs() with something like > >> > >> $ PGDATA="/my/data/" postmaster -k "-D" -S 100 & > >> $ pg_ctl -D /my/data/ restart > >> > >> that example causes pg_ctl to fail to start the server after stopping > >> it, although perhaps you could even trick the server into starting > >> with the wrong options. Of course, similar problems exists today in > >> other cases, such as with the relative paths issue this patch is > >> trying to address, or a datadir containing embedded quotes. > >> > >> I am eager to see the relative paths issue fixed, but maybe we need to > >> bite the bullet and sort out the escaping of command-line options in > >> the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \" > >> quote" can consistently be used by pg_ctl {start|stop|restart} before > >> we can fix this wart. > > > >Where are we on this patch? > > > >-- > > Bruce Momjian <bruce@momjian.us> http://momjian.us > > EnterpriseDB http://enterprisedb.com > > > > + Everyone has their own god. + > > > > > >-- > >Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > >To make changes to your subscription: > >http://www.postgresql.org/mailpref/pgsql-hackers > > > > > > Hi, I encountered the same problem. > I want to know is there a final conclusion? > thank you very much! It seems to me agrouding on parsing issue. We haven't find a way to parse the content of postmaster.opt properly. 1. For escaped option arguments, we can't find where directory name ends. "-D" "/tmp/here's a \" quote" 2. We need to distinguish option names and arguments. "-k" "-D" # "-D" is an arguemnt, not a option name. 3. This is not mentioned here, but getopt accepts "merged" (I'm not sure what to call it.) short options. "-iD" "/hoge" # equivalent to "-i" "-D" "hoge" We need to either let pg_ctl reparse the commandline the same way with postmaster or let postmaster normalize and/or markup the content of postmaster.opts so that pg_ctl can read it desired way. That can be as attached, but the change seems a bit too big.. regards. -- Kyotaro Horiguchi NTT Open Source Software Center diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 5b5fc97c72..0650cc10e8 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -560,6 +560,45 @@ int postmaster_alive_fds[2] = {-1, -1}; HANDLE PostmasterHandle; #endif +char *norm_args = NULL; /* normalized arguments */ +char *norm_args_tail = NULL; +int norm_args_len = 0; + +static void +add_norm_argument(int opt, char *value) +{ + int valuelen = 0; + + if (norm_args_len == 0) + { + norm_args_len = 128; + norm_args = malloc(norm_args_len); + norm_args_tail = norm_args; + } + + if (opt == 0) + { + *norm_args_tail++ = '\0'; /* terminator */ + return; + } + + if (value) + valuelen = strlen(value) + 3; /* _\"val\"*/ + + /* expand buffer as needed */ + while (norm_args_tail - norm_args + 4 /* \"-x\" */ + valuelen + 1 + > norm_args_len) + norm_args_len *= 2; + norm_args = realloc(norm_args, norm_args_len); + + *norm_args_tail++ = '\1'; /* delimiter */ + + if (value != NULL) + norm_args_tail += sprintf(norm_args_tail, "\"-%c\" \"%s\"", opt, value); + else + norm_args_tail += sprintf(norm_args_tail, "\"-%c\"", opt); +} + /* * Postmaster main entry point */ @@ -680,6 +719,8 @@ PostmasterMain(int argc, char *argv[]) */ while ((opt = getopt(argc, argv, "B:bc:C:D:d:EeFf:h:ijk:lN:nOo:Pp:r:S:sTt:W:-:")) != -1) { + add_norm_argument(opt, optarg); + switch (opt) { case 'B': @@ -850,6 +891,9 @@ PostmasterMain(int argc, char *argv[]) } } + /* terminate normalized arguemnt list */ + add_norm_argument(0, NULL); + /* * Postmaster accepts no non-option switch arguments. */ @@ -5666,7 +5710,6 @@ static bool CreateOptsFile(int argc, char *argv[], char *fullprogname) { FILE *fp; - int i; #define OPTS_FILE "postmaster.opts" @@ -5677,8 +5720,8 @@ CreateOptsFile(int argc, char *argv[], char *fullprogname) } fprintf(fp, "%s", fullprogname); - for (i = 1; i < argc; i++) - fprintf(fp, " \"%s\"", argv[i]); + if (norm_args) + fprintf(fp, "%s", norm_args); fputs("\n", fp); if (fclose(fp)) diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c index 1cdc3ebaa3..b4ccaf224f 100644 --- a/src/bin/pg_ctl/pg_ctl.c +++ b/src/bin/pg_ctl/pg_ctl.c @@ -755,10 +755,37 @@ read_post_opts(void) * Are we at the first option, as defined by space and * double-quote? */ - if ((arg1 = strstr(optline, " \"")) != NULL) + if ((arg1 = strstr(optline, "\1\"")) != NULL) { + char *pto; + char *pfrom; + *arg1 = '\0'; /* terminate so we get only program name */ post_opts = pg_strdup(arg1 + 1); /* point past whitespace */ + + pto = pfrom = post_opts; + while (*pfrom) + { + if (*pfrom != '\1') + { + *pto++ = *pfrom++; + continue; + } + + pfrom++; + + /* Remove -D optsion if we have a replacment */ + if (pgdata_opt && strncmp(pfrom, "\"-D\"", 4) == 0) + { + /* Skip -D option */ + while (*pfrom && *pfrom != '\1') pfrom++; + continue; + } + + /* replace '\1' with a space */ + *pto++ = ' '; + } + *pto = 0; } if (exec_path == NULL) exec_path = pg_strdup(optline); @@ -870,8 +897,8 @@ do_start(void) read_post_opts(); - /* No -D or -D already added during server start */ - if (ctl_command == RESTART_COMMAND || pgdata_opt == NULL) + /* Use "" for printf safety */ + if (pgdata_opt == NULL) pgdata_opt = ""; if (exec_path == NULL)
On 07/01/13 20:10, Josh Kupershmidt wrote: > I am eager to see the relative paths issue fixed, but maybe we need to > bite the bullet and sort out the escaping of command-line options in > the rest of pg_ctl first, so that a DataDir like "/tmp/here's a \" > quote" can consistently be used by pg_ctl {start|stop|restart} before > we can fix this wart. It was timely to see this thread recently revived, as I had only just recently needed to contend with the same escaping issue while writing a PostgresNode-like test harness for PL/Java, where I discovered I have to pass -o values pre-transformed to pg_ctl, and even have to do that platform-sensitively, to pre-transform them according to the rules for Bourne shell or those for cmd.exe. I looked at the history of that code in pg_ctl and saw that it went all the way back, so I assumed that any proposal to fix it would probably be met with "it has always been that way and anybody calling it with arbitrary arguments must be pre-transforming them anyway and it would be bad to break that." (And anyway, my test harness thing is now yet one more thing that depends on the current behavior.) But would it be worthwhile to perhaps make a start, add an option (non-default at first) that changes to an implementation that passes values transparently and isn't injection-prone? (I use "injection-prone" not because I'd be especially concerned about command injection by anybody who can already run pg_ctl, just because it's an economical way to describe what pg_ctl currently does.) Regards, -Chap