Re: [PATCHv4 1/4] notes: don't leak memory in git_config_get_notes_strategy

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2016, 16:12 UTC
Message-ID: <xmqqh9fljojo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPig+cQ=83k-of=jP1r6bYqsYrUhiDKhR3_Bm-SfYndn+sL=Dg@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 7 quoted lines

> Meh. Rather than reverting the git_config_get_value(), it would have
> been just as easy and safer (less chance of a future change
> re-introducing a leak) if you had just inserted the necessary check
> here:
>
>     if (!value)
>         return  config_error_nonbool(key);

Yup, sounds much more sensible fix that is useful in the longer term (and avoids one unnecessary strdup()).

Show 12 quoted lines

>
> But, perhaps it's not worth the patch churn at this point...
>
>>         if (parse_notes_merge_strategy(value, strategy))
>>                 git_die_config(key, "unknown notes merge strategy %s", value);
>>
>> +       free(value);
>>         return 0;
>>  }
>>
>> --
>> 2.5.0.264.gc776916.dirty