Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 21 Nov 2006 12:50:53 -0600 (CST)
From:      "Sean C. Farley" <sean-freebsd@farley.org>
To:        Stefan Esser <se@freebsd.org>
Cc:        freebsd-current@freebsd.org
Subject:   Re: Fix for memory leak in setenv/unsetenv
Message-ID:  <20061121124316.Q4388@baba.farley.org>
In-Reply-To: <20061019130354.D92319@thor.farley.org>
References:  <20061006200320.T1063@baba.farley.org> <200610111427.42195.jhb@freebsd.org> <20061018211005.L1466@baba.farley.org> <200610191032.29232.jhb@freebsd.org> <20061019112601.J91957@thor.farley.org> <4537B53A.3080208@FreeBSD.org> <20061019130354.D92319@thor.farley.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 19 Oct 2006, Sean C. Farley wrote:

> On Thu, 19 Oct 2006, Stefan Esser wrote:
> > I've got to admit, that I have not looked your patch, but the only
> > drawback seems to be that the last instance of a variable in the
> > environment space has to be located in getenv() (maximizing the
> > search time ...).
> 
> Actually, the first active variable found is returned by getenv() even
> if another would be found later.  This does make me think that if
> changed the way the environment variable array was built to only
> contain the first instance of each variable instead of all instances
> then a search by getenv() from the end of the array backwards would be
> faster.  A cheap alternative is to create the array in reverse.
> 
> > Always using the last allocated (largest) slot for storage of new
> > values of environment variables would result in nearly reasonable
> > behavior. A cached pointer does either point to the value of the
> > variable at the time of the getenv(), or to the last value assigned
> > to the environment variable that does not exceed the allocated size.
> 
> I will look at changing it.

This is a resend from two weeks ago with an edit about using a
constructor.

I changed it to create the array in reverse and to search it from the
end.

I played with crt1.c to have it initialize the new environment and clean
it up via atexit().  The feeling I get is that this may be disliked
since malloc_init() is not called here.  Would there be a reason not to
have things such as memory allocation and an environment (such as I have
been doing) initialized and/or cleaned up here?  How about using a
constructor (__attribute__((constructor)))?

Would there be anything else I should code to have this be a viable
replacement to the current implementation of the setenv-family of
functions?

Sean
-- 
sean-freebsd@farley.org



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