Re: Fix for pg_upgrade's forcing pg_controldata into English

Bruce Momjian <bruce@momjian.us>

From: Bruce Momjian <bruce@momjian.us>
To: Bruce Momjian <bruce@momjian.us>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Alvaro Herrera <alvherre@commandprompt.com>, Greg Sabino Mullane <greg@turnstep.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2010-09-07T14:11:09Z
Lists: pgsql-hackers
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce@momjian.us> writes:
> > > Tom Lane wrote:
> > >> I certainly hope that pg_regress isn't freeing the strings it passes
> > >> to putenv() ...
> > 
> > > pg_regress does not restore these settings (it says with C/English) so
> > > the code is different.
> > 
> > That's not what I'm on about.  You're trashing strings that are part of
> > the live environment.  It might accidentally fail to fail for you, if
> > your version of free() doesn't immediately clobber the released storage,
> > but it's still broken.  Read the putenv() man page.
> > 
> > + #ifndef WIN32
> > + 		char	   *envstr = (char *) pg_malloc(ctx, strlen(var) +
> > + 							strlen(val) + 1);
> > + 
> > + 		sprintf(envstr, "%s=%s", var, val);
> > + 		putenv(envstr);
> > + 		pg_free(envstr);
> >                 ^^^^^^^^^^^^^^^^
> > + #else
> > + 		SetEnvironmentVariableA(var, val);
> > + #endif
> > 
> > The fact that there is no such free() in pg_regress is not an oversight
> > or shortcut.
> 
> Interesting.  I did not know this and it was not clear from my manual
> page or FreeBSD's manual page, but Linux clearly does this.
> 
> Updated patch attached.

Applied to HEAD and 9.0.X.  Thanks for the ideas/review.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +