On 11/20/2011 11:07 AM, Josh Kupershmidt wrote:
On Wed, Nov 2, 2011 at 5:36 PM, Andrew Dunstan<and...@dunslane.net> wrote:
Updated patch is attached - adding to Nov commitfest.
Review of the v2 patch:
* Submission Review
Patch applies with some fuzz and builds without warnings. I noticed
some tab characters being used in psql-ref.sgml where they shouldn't
be.
Fixed.
* Coding Review / Nitpicks
The patch implements \setenv via calls to unsetenv() and putenv(). As
the comment notes,
| That means we
| leak a bit of memory here, but not enough to worry about.
which seems true; the man page basically says there's nothing to be
done about the situation.
The calls to putenv() and unsetenv() are done without any real input
checking. So this admittedly-pathological case produces surprising
results:
test=# \setenv foo=bar baz
test=# \! echo $foo
bar=baz
Perhaps 'envvar' arguments with a '=' in the argument should just be
disallowed.
Done that way.
Also, should the malloc() of newval just use pg_malloc() instead?
Yes, also done.
Revised patch attached.
cheers
andrew
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 01f57c4..39193d5 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2242,6 +2242,24 @@ lo_import 152801
<varlistentry>
+ <term><literal>\setenv [ <replaceable class="parameter">name</replaceable> [ <replaceable class="parameter">value</replaceable> ] ]</literal></term>
+
+ <listitem>
+ <para>
+ Sets the environment variable <replaceable
+ class="parameter">name</replaceable> to <replaceable
+ class="parameter">value</replaceable>, or if the
+ <replaceable class="parameter">value</replaceable> is
+ not supplied, unsets the environment variable. Example:
+<programlisting>
+foo=> <userinput>\setenv PAGER less</userinput>
+foo=> <userinput>\setenv LESS -imx4F</userinput>
+</programlisting>
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>\sf[+] <replaceable class="parameter">function_description</> </literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9cc73be..b5331aa 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1110,6 +1110,56 @@ exec_command(const char *cmd,
free(opt0);
}
+
+ /* \setenv -- set environment command */
+ else if (strcmp(cmd, "setenv") == 0)
+ {
+ char *envvar = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+ char *envval = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ if (!envvar)
+ {
+ psql_error("\\%s: missing required argument\n", cmd);
+ success = false;
+ }
+ else if (strchr(envvar,'=') != NULL)
+ {
+ psql_error("\\%s: environment variable name must not contain '='\n",
+ cmd);
+ success = false;
+ }
+ else if (!envval)
+ {
+ /* No argument - unset the environment variable */
+ unsetenv(envvar);
+ success = true;
+ }
+ else
+ {
+ /* Set variable to the value of the next argument */
+ int len = strlen(envvar) + strlen(envval) + 1;
+ char *newval = pg_malloc(len + 1);
+
+ if (!newval)
+ {
+ psql_error("out of memory\n");
+ exit(EXIT_FAILURE);
+ }
+ snprintf(newval, len+1, "%s=%s", envvar, envval);
+ putenv(newval);
+ success = true;
+ /*
+ * Do not free newval here, it will screw up the environment
+ * if you do. See putenv man page for details. That means we
+ * leak a bit of memory here, but not enough to worry about.
+ */
+ }
+ free(envvar);
+ free(envval);
+ }
+
/* \sf -- show a function's source code */
else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0)
{
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 4649e94..20fb5d7 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -158,7 +158,7 @@ slashUsage(unsigned short int pager)
{
FILE *output;
- output = PageOutput(93, pager);
+ output = PageOutput(94, pager);
/* if you add/remove a line here, change the row count above */
@@ -257,6 +257,7 @@ slashUsage(unsigned short int pager)
fprintf(output, _("Operating System\n"));
fprintf(output, _(" \\cd [DIR] change the current working directory\n"));
+ fprintf(output, _(" \\setenv NAME [VALUE] set environment variable, or unset it if no value provided\n"));
fprintf(output, _(" \\timing [on|off] toggle timing of commands (currently %s)\n"),
ON(pset.timing));
fprintf(output, _(" \\! [COMMAND] execute command in shell or start interactive shell\n"));
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
