Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 19 Apr 2007 18:06:43 -0700
From:      Julian Elischer <julian@elischer.org>
To:        "Sean C. Farley" <sean-freebsd@farley.org>
Cc:        freebsd-current@freebsd.org
Subject:   Re: Fix for memory leak in setenv/unsetenv (take 2)
Message-ID:  <46281223.8060800@elischer.org>
In-Reply-To: <20070419175902.R44041@thor.farley.org>
References:  <20070419175902.R44041@thor.farley.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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@ 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/




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?46281223.8060800>