Duy Nguyen <pclouds@xxxxxxxxx> writes:
> On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano <gitster@xxxxxxxxx> 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.
Just to make sure you did not misunderstand, the 4 (four) in my
message is not about "4 calls among 143 are unsafe".
It was referring to the number of rotating slots your patch defined,
which means
val = getenv("FOO");
... random other code ...
use(val)
is safe only if random other code makes less than 4 getenv() calls.
I didn't verify all of the call sites. It needs to be done with or
without your wrapper patch. Without your wrapper, the validation
needs to make sure "random other code" above does not make any getenv()
call. With your wrapper, the validation needs to make sure "random
other code" above does not make more than 4 such calls. My point
was that the effort needed for both are about the same, so your
wrapper does not buy us much.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html