Subject: Re: [PATCH v3 4/4] restore_env(): free the saved environment variable once we are done

From: Junio C Hamano <[email-masked]>
Date: Tue, 02 Feb 2016 18:19:32 -0800

Eric Sunshine <[email-masked]> writes:

>> diff --git a/git.c b/git.c
>> @@ -54,10 +54,12 @@ static void restore_env(int external_alias)
>>                 if (external_alias &&
>>                     !strcmp(env_names[i], GIT_PREFIX_ENVIRONMENT))
>>                         continue;
>> -               if (orig_env[i])
>> +               if (orig_env[i]) {
>>                         setenv(env_names[i], orig_env[i], 1);
>> -               else
>> +                       free(orig_env[i]);
>
> Now that this is "well-protected"[1] against incorrect nesting, you
> don't worry about the dangling pointers in static orig_env[], right?
> (The same for the dangling pointer in static 'orig_cwd' after being
> freed a bit earlier in this function, correct?)

Correct.

I do not think we follow a style that requires "after freeing memory
pointed at by a variable, the variable must be assigned NULL".
Would it be a good idea?  I do not see a point in it.

Thanks.