On Wed, Sep 20, 2017 at 09:47:24PM +0200, Martin Ågren wrote: > Instead of conditionally freeing `rev.pending.objects`, just call > `object_array_clear()` on `rev.pending`. This means we don't poke as > much into the implementation, which is already a good thing, but also > that we free the individual entries as well, thereby fixing a > memory-leak. > > Signed-off-by: Martin Ågren <martin.agren@xxxxxxxxx> > --- > diff-lib.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/diff-lib.c b/diff-lib.c > index 2a52b07..4e0980c 100644 > --- a/diff-lib.c > +++ b/diff-lib.c > @@ -549,7 +549,6 @@ int index_differs_from(const char *def, int diff_flags, > rev.diffopt.flags |= diff_flags; > rev.diffopt.ita_invisible_in_index = ita_invisible_in_index; > run_diff_index(&rev, 1); > - if (rev.pending.alloc) > - free(rev.pending.objects); > + object_array_clear(&rev.pending); Looks good. A similar bug was the exact reason for adding the function in 46be82312. I did a grep for 'free.*\.objects' to see if there were other cases. There are some hits. E.g., the one in orphaned_commit_warning(). It does something funny with setting revs.leak_pending. But I _think_ after the whole thing is done and we call free(refs.objects), that probably ought to be an object_array_clear(). As I suspect you're working your way through leak-checker results, I'm OK if you want to punt on digging into more cases for now and just fix ones that the tool has identified as real leaks. -Peff
