Re: [PATCH 1/3] bisect: fix memory leak and document `find_bisection()`

["Martin Ågren" <martin.agren@gmail.com>]

On 2 November 2017 at 05:54, Junio C Hamano <gitster@pobox.com> wrote:
> Martin Ågren <martin.agren@gmail.com> writes:
>
>> `find_bisection()` rebuilds the commit list it is given by reversing it
>> and skipping uninteresting commits. The uninteresting list entries are
>> leaked. Free them to fix the leak.
>>
>> While we're here and understand what's going on, document the function.
>> In particular, make sure to document that the original list should not
>> be examined by the caller.
>
> Good.  Thanks.
>
> I notice that this has only two callers and both of them do
>
>         revs.commits = find_bisection(revs.commits, ...);
>
> I wonder if updating its calling convention to
>
>         (void) find_bisection(&revs.commits, ...);
>
> makes sense.  This is obviously outside the scope of this patch.

I think it would. I considered it briefly but punted, though I don't
really understand why now. In hindsight, it would have saved me some
work, since I wouldn't have had to carefully document that the original
list mustn't be examined. I'll let this simmer for a few days in case
other comments turn up, then do a v2 with a preparatory patch that
switches the calling convention as you suggested.

Thanks.