Re: Environment handling broken in /bin/sh with changes to {get,set,put}env()

On Sat, 7 Jul 2007, Andrey Chernov wrote:

> On Sat, Jul 07, 2007 at 05:09:00PM +0400, Andrey Chernov wrote:
>> Well, I see. You try to keep envVars[] between environ switch by that
>> way.

My intention is have all allocated memory tracked.  This way all memory
can be freed explicitly.  In turn, this allows for easier debugging of
*env() functions or any programmers using these functions.

Besides, I wrote the code on July 4th.  It was born to be free().  :)

>> But it still look complicated and probably gains nothing. I.e. will
>> be much _faster_ just free envVars[] (but not variables themselfs)
>> and allow build_env() to calloc() new array for envVars and fills it
>> from new environ. It is surely faster than calling setenv() for each
>> variable just for sake of keepeng once allocated envVars[].

I feel the change is easy to follow since it is reusing existing
functions.  Compared to other sections of code, this part is easy.  :)

I agree that it would be faster for a subset of an existing environ.  On
the other hand, in the case of emptying the environment, my method would
be faster since no deallocation, allocation nor setenv() calls would be
called assuming putenv() was not used.  I could try a few tests to see
what is faster in which case, but I do not think environ changes happen
often enough to make speed a factor.

>> Moreover, environ switch commonly used to switch from large environ
>> to smaller one (or to empty one), so the rest of old envVars[] array
>> would keep unneccessary allocation.

If the new environ only contains a subset with the same values of an
existing environ, then the total bytes will not increase.  No
(de)allocations functions would be called.  freeing envVars and starting
anew will result in memory allocations for envVars and every variable
within the new environ.

> BTW, if you just free(envVars) when environ switch is detected, there
> is no needs to __remove_putenv() (big slowdown) in the __clean_env()
> too.

I just changed it to only call __remove_putenv() during a merge.

Sean
-- 
scf_at_FreeBSD.org