Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites

Duy Nguyen [off-list ref] writes:

Maybe we could all this into a wrapper? If getenv() here has a
problem, many other places may have the same problem too. This
simplifies the change. But one has to check that getenv() must not be
used in threaded code.

That needs to be done regardless, if we care; POSIX explicitly says
getenv() need not be thread-safe.

I personally do not think a wrapper with limited slots is a healthy
direction to go.  Most places we use getenv() do not let the return
value live across their scope, and those that do should explicitly
copy the value away.  It's between validating that there is _no_ *env()
calls in the codepath between a getenv() call and the use of its
return value, and validating that there is at most 4 such calls there.
The former is much easier to verify and maintain, I think.

On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano [off-list ref] wrote:

I personally do not think a wrapper with limited slots is a healthy
direction to go.  Most places we use getenv() do not let the return
value live across their scope, and those that do should explicitly
copy the value away.  It's between validating that there is _no_ *env()
calls in the codepath between a getenv() call and the use of its
return value, and validating that there is at most 4 such calls there.
The former is much easier to verify and maintain, I think.

I did not look carefully and was scared of 143 getenv calls. But with
about 4 calls, yes it's best to do without the wrapper.
-- 
Duy