Re: psql setenv command

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
Cc: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: psql setenv command
Date: 2011-11-26 16:02:10
Message-ID: 4ED10D82.9050105@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 11/20/2011 11:07 AM, Josh Kupershmidt wrote:
> On Wed, Nov 2, 2011 at 5:36 PM, Andrew Dunstan<andrew(at)dunslane(dot)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

Attachment Content-Type Size
setenv.patch text/x-patch 3.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Steve Singer 2011-11-26 16:21:14 Re: plpython SPI cursors
Previous Message Tom Lane 2011-11-26 15:58:39 Re: [COMMITTERS] pgsql: Modify pg_dump to use error-free memory allocation macros. This