Re: [PATCH] pretty.c: make git_pretty_formats_config return -1 on git_config_string failure

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Aug 4, 2014, 19:49 UTC
Message-ID: <vpqy4v4avc0.fsf@anie.imag.fr>
In-Reply-To: <CAPig+cRo05mG9yeU61VSjAvXWRHU9soaaH-Cv7MKoxZ=it15Rw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 15 quoted lines

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

Not sure it's worth the trouble: the bug corresponds to a mis-application of a pattern used in tens of places in Git's code (basically, each call to git_config_string, 50 callsites). Testing this particular case does not ensure non-regression, and testing all occurences of the pattern would be overkill IMHO.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/