Re: Fix for memory leak in setenv/unsetenv (take 2)

From: Julian Elischer <julian_at_elischer.org>
Date: Thu, 19 Apr 2007 18:06:43 -0700
Sean C. Farley wrote:
> I have a new patch[1] that fixes memory leaks caused by repeated calls
> to setenv() with varying-sized values or unsetenv().  The web page has a
> better description about it.
> 

I vaguely remember that several people have tried to fix this before,
but that fixing it actually breaks some ABI..

I may be wrong but maybe others remember better what the issue was.
I believe that the end result was that it was considered better to leak
memory than break the posix specified ABI in some way.

It's very vague in my memory though.


> I tested this with -STABLE with various applications.  Also, to help
> check it further, I wrote a few regression tests for it.
> 
> Notes/questions I have:
> 1. Would making it more IEEE Std 1003.1-compliant be desired?
>    a. FreeBSD's setenv() allows an '=' in the name and value to comply
>       with other standards while this standard disallows it.
>    b. FreeBSD's unsetenv() does not have a return value while this
>       standard returns an int.  For the changes I made, this would be
>       beneficial.
> 2. The "feature"--it is under the BUGS section :)--of keeping all
>    pointers returned by previous calls to getenv() valid regardless of
>    any calls to setenv() is still there.  Keeping this requirement
>    prevents a complete fix.
> 3. I previously thought about having the implementation initialize
>    itself upon the library loading or called within crt1.c.  The problem
>    with a library-load time method is that changes made to the environ
>    variable do not persist at the execution of main().  The issue with
>    calling it within crt1.c is that FreeBSD's malloc() would need to
>    call __findenv_environ() to find MALLOC_OPTIONS when it is
>    initialized to prevent a recursion into each other.  jasone_at_ helped
>    me look at this.
> 4. Ignore dmalloc defines for now; they will be removed.
> 
> Basically, the patch contains sysenv.c and a change to the Makefile to
> remove building of putenv.c and setenv.c.  To increase the speed of
> putenv(), I have moved it into sysenv.c to make use of some internal
> (static) functions.  I have just started with it, but I may leave it
> alone.
> 
> Please let me know if you see any bugs.
> 
> Sean
>   1. http://www.farley.org/freebsd/tmp/setenv-6/
Received on Thu Apr 19 2007 - 23:06:36 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:09 UTC