Subject: Re: [PATCH v2 11/24] submodule--helper: fix "sm_path" and other "module_cb_list" leaks

From: Junio C Hamano <[email-masked]>
Date: Wed, 20 Jul 2022 15:35:41 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> -	const char *sm_path;
> +	char *sm_path;
>  };
>  #define MODULE_CB_INIT { 0 }
> +static void module_cb_release(struct module_cb *mcb)
> +{
> +	free(mcb->sm_path);
> +}

It would be nice to have a blank line before the new helper
function, just like you did for the next hunk.

>  struct module_cb_list {
>  	struct module_cb **entries;
> @@ -838,6 +842,19 @@ struct module_cb_list {
>  };
>  #define MODULE_CB_LIST_INIT { 0 }
>  
> +static void module_cb_list_release(struct module_cb_list *mcbl)
> +{
> +	int i;
> +
> +	for (i = 0; i < mcbl->nr; i++) {
> +		struct module_cb *mcb = mcbl->entries[i];
> +
> +		module_cb_release(mcb);
> +		free(mcb);
> +	}
> +	free(mcbl->entries);
> +}

OK, this was populated in the diff callback function and each
element of the list->entries[] array is a module_cb data, which has
its own release() helper above.

Looking good.

>  struct summary_cb {
>  	int argc;
>  	const char **argv;
> @@ -1181,6 +1198,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
>  cleanup:
>  	strvec_clear(&diff_args);
>  	release_revisions(&rev);
> +	module_cb_list_release(&list);
>  	return ret;
>  }