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

On Thursday 19 April 2007 09:06:43 pm Julian Elischer wrote:
> 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.

Sean's changes don't break the ABI, but they do make more efficient use of the 
memory.  Specifically, setenv() will reuse the variable part of the 
name=value pair if the new value will fit in the same or less space as the 
old value, and it uses strlen() to determine this.  What Sean's patches fix 
is this case:

	setenv("FOO", "88888888");	/* 8 char string as value */
	setenv("FOO", "4444");		/* 4 chars, reuses 8 char buffer */
	setenv("FOO", "88888888");	/* setenv thinks buffer is 4 char,
					   so allocates a new one for 8 char */

Sean keeps track of the true allocated length of each string, so with his 
change, you the 3rd setenv will reuse the existing buffer instead of 
allocating a new one.  At my previous job, we space padded timezone values 
out to a fixed value to workaround the problem of alternating between 
different length timezone names and thus leaking memory endlessly.

Since setenv(3) already overwrites the value with a new one in some cases, 
Sean's patch doesn't change behavior, it just allows it to reuse the old 
buffer more often than it does now.  This really should go in, I just haven't 
had time to review it in detail.  If someone else wants to help and step up 
to the plate to do that, that would be appreciated. :)

-- 
John Baldwin