Re: Fix for memory leak in setenv/unsetenv

On Wed, 11 Oct 2006, John Baldwin wrote:
> On Wednesday 11 October 2006 12:15, Sean C. Farley wrote:
>> On Tue, 10 Oct 2006, John Baldwin wrote:
>>> This still won't work.  The reason for the intentional leak is
>>> because of this code sequence:
>>>
>>> 	char *a;
>>>
>>> 	setenv("FOO", "0", 1);
>>> 	a = getenv("FOO");
>>> 	setenv("FOO", "bar", 1);
>>> 	printf("FOO was %s\n", a);
>>>
>>> With the memory leak fixed this will use free'd memory.  While this
>>> code may seem weird in a program, it actually is quite possible for
>>> a library to read and cache the value of an environment variable.
>>> If you didn't leave the leak around, the library could cause a crash
>>> if the main program (or another library) changed the environment
>>> variable the first library had a cached pointer to the value of.

<snip>

> Yeah, but it doesn't crash is the point actually.  The pointer is
> still valid, though it may be overwritten with a newer value, it's
> still valid and a library can reliably doing getenv() and that pointer
> will always point to some value of that variable, but it won't ever
> point to anything else.

<snip>

> Part of the problem is that we have no way to notify consumers of an
> environment variable when its value is changed.  Alternatively, we
> could add a different variant of getenv that required the user to
> supply the buffer, but that's not the API we have.

OK.  I decided to fix the memory leak as well as keep backward
compatibility.  The result is on my site tar'd[1] and extracted[2].  It
still needs some touch-ups, but it works.  It is even faster than the
current implementation when I compared "hungry" and "lean" (main.c
without the sleep() call).

Features:
1. No memory leak.  :)
2. Able to be cleaned up:  __clean_environ()
3. Backward compatible.
4. Faster.

Nice changes to make (but unnecessary):
1. Call __build_env() just after a process starts instead of checking
    within the public env functions.
2. Call __rebuild_environ() just before uses of environ within libc
    (i.e., execl(), execv(), execvP(), popen(), _init_tls()) instead of
    after every getenv(), setenv(), unsetenv().
3. Call __clean_environ() at process exit.  This allows for fewer leaks
    when a developer is debugging his code.

Sean
   1. http://www.farley.org/freebsd/tmp/setenv-4.tar.bz2
   2. http://www.farley.org/freebsd/tmp/setenv-4/
-- 
sean-freebsd_at_farley.org