From: Derrick Stolee <stolee@gmail.com>
Date: Fri, 08 Jun 2018 12:48:32 GMT
Subject: Re: [PATCH v5 18/21] commit-graph: use string-list API for input
Message-ID: <4c5982b2-15a0-9172-c115-e78afe0b4daa@gmail.com>
In-Reply-To: <548f9285-da68-478c-1423-a9b9b5021d97@gmail.com>

On 6/6/2018 8:45 AM, Derrick Stolee wrote:
> On 6/6/2018 8:26 AM, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Jun 06 2018, Derrick Stolee wrote:
>>
>>> On 6/6/2018 8:11 AM, Ævar Arnfjörð Bjarmason wrote:
>>>> On Wed, Jun 06 2018, Derrick Stolee wrote:
>>>>
>>>>> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
>>>>>
>>>>> +    string_list_clear(&lines, 0);
>>>>>        return 0;
>>>>>    }
>>>> This results in an invalid free() & segfault because you're freeing
>>>> &lines which may not have been allocated by string_list_init().
>>> Good point. Did my tests not catch this? (seems it requires calling
>>> `git commit-graph write` with no `--stdin-packs` or
>>> `--stdin-commits`).
>> Most of your tests (t5318-commit-graph.sh) segfaulted, but presumably
>> you're on a more forgiving compiler/platform/options. I compiled with
>> -O0 -g on clang 4.0.1-8 + Debian testing.
>
> I appreciate the extra platform testing. I'm using GCC on Ubuntu (gcc 
> (Ubuntu 7.3.0-16ubuntu3) 7.3.0).

While the errors didn't repro on my box, they did on the VSTS Linux 
build machines [1]. I created an internal PR just so I could run those 
tests (and on our OSX agents which use clang) and so I can verify I 
fixed the situation. I'll send a v6 soon so Junio only picks up a 
version that succeeds in tests.

Thanks,
-Stolee

[1] 
https://github.com/Microsoft/vsts-agent-docker/blob/master/ubuntu/16.04/standard/Dockerfile
     What's installed on a VSTS Linux build agent