Re: Fix for pg_upgrade's forcing pg_controldata into English

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Greg Sabino Mullane <greg(at)turnstep(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix for pg_upgrade's forcing pg_controldata into English
Date: 2010-09-04 17:50:08
Message-ID: 201009041750.o84Ho8814713@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)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.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachment Content-Type Size
/pgpatches/pg_upgrade text/x-diff 5.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2010-09-04 18:16:21 Re: 9.1alpha1 bundled -- please verify
Previous Message Dave Page 2010-09-04 17:35:01 Re: 9.1alpha1 bundled -- please verify