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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 3, 2016, 02:19 UTC
Message-ID: <xmqqy4b2o7rf.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cT8PhJU=9aS8NvkN9wx6imtACOS9rkgDJeJpN=CGikd7Q@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 15 quoted lines

>> 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.