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