Re: [PATCH v2 11/24] submodule--helper: fix "sm_path" and other "module_cb_list" leaks
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 20, 2022, 22:35 UTC
- Message-ID
- <xmqqr12f1kg2.fsf@gitster.g>
- In-Reply-To
- <patch-v2-11.24-c25b55c9528-20220719T204458Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 8 quoted lines
> - 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.
Show 18 quoted lines
> 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.
Show 10 quoted lines
> 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;
> }