Re: [PATCH] ref-filter: fix memory leak in `free_array_item()`

[Junio C Hamano <gitster@pobox.com>]

Martin Ågren <martin.agren@gmail.com> writes:

> We treat the `value` pointer as a pointer to a struct and free its `s`
> field. But `value` is in fact an array of structs. As a result, we only
> free the first `s` out of `used_atom_cnt`-many and leak the rest. Make
> sure we free all items in `value`.

Thanks for spotting.  We do allocate an array of used_atom_cnt elements
in populate_value() and we need to free them all.

> In the caller, `ref_array_clear()`, this means we need to be careful not
> to zero `used_atom_cnt` until after we've called `free_array_item()`. We
> could move just a single line, but let's keep related things close
> together instead, by first handling `array`, then `used_atom`.

Yup.  Looking good.

Thanks.

>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
>  ref-filter.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 791f0648a6..1c1a2af880 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2105,7 +2105,9 @@ static void free_array_item(struct ref_array_item *item)
>  {
>  	free((char *)item->symref);
>  	if (item->value) {
> -		free((char *)item->value->s);
> +		int i;
> +		for (i = 0; i < used_atom_cnt; i++)
> +			free((char *)item->value[i].s);
>  		free(item->value);
>  	}
>  	free(item);
> @@ -2116,14 +2118,16 @@ void ref_array_clear(struct ref_array *array)
>  {
>  	int i;
>  
> -	for (i = 0; i < used_atom_cnt; i++)
> -		free((char *)used_atom[i].name);
> -	FREE_AND_NULL(used_atom);
> -	used_atom_cnt = 0;
>  	for (i = 0; i < array->nr; i++)
>  		free_array_item(array->items[i]);
>  	FREE_AND_NULL(array->items);
>  	array->nr = array->alloc = 0;
> +
> +	for (i = 0; i < used_atom_cnt; i++)
> +		free((char *)used_atom[i].name);
> +	FREE_AND_NULL(used_atom);
> +	used_atom_cnt = 0;
> +
>  	if (ref_to_worktree_map.worktrees) {
>  		hashmap_free(&(ref_to_worktree_map.map), 1);
>  		free_worktrees(ref_to_worktree_map.worktrees);