Re: [PATCH 1/3] bisect: fix memory leak and document `find_bisection()`
- Date: Thu, 2 Nov 2017 11:47:13 +0100
- From: Martin Ågren <martin.agren@xxxxxxxxx>
- Subject: Re: [PATCH 1/3] bisect: fix memory leak and document `find_bisection()`
On 2 November 2017 at 05:54, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Martin Ågren <martin.agren@xxxxxxxxx> 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.