Re: [PATCH] diff-lib: clear `pending` object-array in `index_differs_from()`

[Jeff King <peff@peff.net>]

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@gmail.com>
> ---
>  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