On Sat, Nov 26, 2011 at 11:02 AM, Andrew Dunstan <and...@dunslane.net> wrote:
>> Also, should the malloc() of newval just use pg_malloc() instead?
>
> Yes, also done.

This bit is unnecessary, since pg_malloc() takes care of the error handling:

+                       if (!newval)
+                       {
+                               psql_error("out of memory\n");
+                               exit(EXIT_FAILURE);
+                       }


Also, the help output for setenv bleeds over an 80-character terminal,
and it seems the rest of the help outputs try to stay under this
limit. And an OCD nitpick: most of the psql-ref.sgml examples show
'testdb' at the prompt, how about we follow along.

 Other than those small gripes, the patch looks fine. Attached is an
updated version to save you some keystrokes.

Josh
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 01f57c4..f97929b 100644
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
*************** lo_import 152801
*** 2242,2247 ****
--- 2242,2265 ----
  
  
        <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>
+ testdb=&gt; <userinput>\setenv PAGER less</userinput>
+ testdb=&gt; <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..e4b4eaa 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 1110,1115 ****
--- 1110,1160 ----
  		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);
+ 
+ 			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..13d8bf6 100644
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
*************** slashUsage(unsigned short int pager)
*** 158,164 ****
  {
  	FILE	   *output;
  
! 	output = PageOutput(93, pager);
  
  	/* if you add/remove a line here, change the row count above */
  
--- 158,164 ----
  {
  	FILE	   *output;
  
! 	output = PageOutput(94, pager);
  
  	/* if you add/remove a line here, change the row count above */
  
*************** slashUsage(unsigned short int pager)
*** 257,262 ****
--- 257,263 ----
  
  	fprintf(output, _("Operating System\n"));
  	fprintf(output, _("  \\cd [DIR]              change the current working directory\n"));
+ 	fprintf(output, _("  \\setenv NAME [VALUE]   set or unset environment variable\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

Reply via email to