From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mon, 04 Aug 2014 18:56:03 GMT
Subject: Re: [PATCH] pretty.c: make git_pretty_formats_config return -1 on git_config_string failure
Message-ID: <CAPig+cRo05mG9yeU61VSjAvXWRHU9soaaH-Cv7MKoxZ=it15Rw@mail.gmail.com>
In-Reply-To: <vpqmwbki7h3.fsf@anie.imag.fr>

On Mon, Aug 4, 2014 at 11:45 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Tanay Abhra <tanayabh@gmail.com> writes:
>
>> `git_pretty_formats_config()` continues without checking git_config_string's
>> return value which can lead to a SEGFAULT.
>
> Indeed, without the patch:
>
> $ git -c pretty.my= log --pretty=my
> error: Missing value for 'pretty.my'
> zsh: segmentation fault  git -c pretty.my= log --pretty=my

This probably should be formalized as a proper test and included with
Tanay's patch.

>> diff --git a/pretty.c b/pretty.c
>> index 3a1da6f..72dbf55 100644
>> --- a/pretty.c
>> +++ b/pretty.c
>> @@ -65,7 +65,9 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c
>>
>>       commit_format->name = xstrdup(name);
>>       commit_format->format = CMIT_FMT_USERFORMAT;
>> -     git_config_string(&fmt, var, value);
>> +     if (git_config_string(&fmt, var, value))
>> +             return -1;
>> +
>
> Ack-ed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> My first thought reading this was "why not rewrite using non-callback
> API?", but this particular call to git_config needs to iterate over
> config keys anyway.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/