Re: Fix for memory leak in setenv/unsetenv

On Thursday 19 October 2006 13:29, Bakul Shah wrote:
> > I preserve the leak, but I also make use of old entries with matching
> > names if their size is big enough.  The maximum size of the value is
> > recorded at allocation.  The code in the source tree uses the strlen()
> > of the current value which can shrink even if the space is available.
> > 
> > Example:
> > setenv("FOO", "BAR1", 1);
> > w = getenv("FOO");
> > setenv("FOO", "BARBAR1", 1);
> > x = getenv("FOO");
> > setenv("FOO", "BAR2", 1);
> > y = getenv("FOO");
> > setenv("FOO", "BARBAR2", 1);
> > z = getenv("FOO");
> > 
> > This will end up with w == y ("BAR2") and y == z ("BAR1").  w was
> > allocated first in the array of variables.  x is then allocated since w
> > is too small.  y finds w within the array and reuses it.  z does the
> > wame with x.  If the larger value had been allocated first, then all
> > setenv() calls would have used the same storage space.  Yes, there is a
> > leak, but flipping back and forth does not cause the leak to keep
> > growing.  Also, there would be no need to space-pad a value to prevent
> > the leak.
> 
> Consider
> 
> > w = getenv("FOO");
> > setenv("FOO", "BARBAR1", 1);
> > x = getenv("FOO");
> 
> The C standard says that the value returned by getenv() shall
> *not* be modified.  AFAIK it doesn't have setenv().
> 
> Which leads me to think that in the above example what w
> points to *must not* change as a result of a subsequent
> setenv() -- w value must be a constant until the program
> terminates or another w = getenv(...) is done.  Note that
> setenv() & getenv() may be called in totally separate
> modules, developed by different people at different times.
> 
> You are stuck with a leak (IMHO a small price to pay for
> cleaner & standard complying semantics -- you can always use
> a conservative GC such as Boehm-Weiser if leak is a major
> problem and you care enough).  I think this is what John
> Baldwin was referring to by "intentional leak".
> 
> Please don't "fix" the leak.

Actually, the current setenv() already breaks that rule in that it
reuses memory if the current string value is not shorter than the new
string value.  Sean is just fixing it to use the real length rather
than strlen().

-- 
John Baldwin