2011/1/11 Colin Guthrie <gmane at colin.guthr.ie>:
>> void pa_set_env(const char *key, const char *value) {
>> ? ? pa_assert(key);
>> ? ? pa_assert(value);
>>
>> ? ? /* This is not thread-safe */
>>
>> ? ? putenv(pa_sprintf_malloc("%s=%s", key, value));
>> }
>
> Hmm, that looks wrong too on first glance, but with a little digging
> (well, man putenv!):
>
> ? ? ? The ?libc4 ?and ?libc5 ?and ?glibc 2.1.2 versions conform to
> SUSv2: the
> ? ? ? pointer string given to putenv() is used. ?In particular, ?this
> string
> ? ? ? becomes ?part ?of ?the ?environment; ?changing it later will
> change the
> ? ? ? environment. ?(Thus, it is an error is to call putenv() with ?an
> ?auto?
> ? ? ? matic ?variable ?as the argument, then return from the calling
> function
> ? ? ? while string ?is ?still ?part ?of ?the ?environment.) ? However,
> ?glibc
> ? ? ? 2.0-2.1.1 ?differs: a copy of the string is used. ?On the one
> hand this
> ? ? ? causes a memory leak, and on the other hand it ?violates ?SUSv2.
> ?This
> ? ? ? has been fixed in glibc 2.1.2.
>
>
> So I guess this was correct back in the olden days but perhaps now we
> should be free'ing it. I'd rather have someone who knows such things
> confirm this first tho' as I'm not really sure!!
Well, on POSIX, setenv would be a better replacement I guess. No
ambiguity about whether to free the string or not.
After a bit of digging, I found that on Windows there is an equivalent:
errno_t _putenv_s(const char *name, const char *value );
That is setenv, without the overwrite parameter.
>> Is that wrong to? What fix would you suggest?
>
> Well, if it turns out correct to do so:
>
> char *t;
> putenv(t = pa_sprintf_malloc("%s=", s));
> pa_xfree(t);
It turns out that Windows actually copies the string, but there is no
API guarantee that it does. Anyway, point is moot considering the
above.
> But I will ask some folk about this as I'm not certain.
>
> FWIW, further up in that file, there is some WIN32 code that uses
> _putenv() with a stack variable. I'm not really sure of the differences
> between putenv() and _putenv() tho' :s
No difference:
http://msdn.microsoft.com/en-us/library/ms235321(v=vs.80).aspx
>
> Col
>
I'll prepare a new patch series with all your points adressed.
Maarten
