Re: [PATCH v2 08/24] submodule--helper: add and use *_release() functions

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Jul 20 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Add release functions for "struct module_list", "struct
>> submodule_update_clone" and "struct update_data".
>
> OK, this does a lot of things, but in short, nobody bothered to free
> module_list (i.e. list of cache entries borrowed from the index to
> represent the paths we are interested in that are gitlinks),
> update_data (which has module_list among other things that do not
> need to be freed), and submodule_update_clone (which has
> update_data, update_clone_data and failed_clones list) in the
> original (in other words, these release helpers had to be invented
> by this patch, not factored out from some codepaths that free them).
>
> I think the earlier part of the patch that deals with module_list is
> correct.  I also think the last one that clears update_data in
> module_update() is correct.

...

> I am not sure about the helper that releases suc, especially how
> suc->update_data is left alone by update_submodules().  Presumably
> the caller is responsible for releasing the resources held by
> update_data member alone, but the interaction makes me feel dirty.
> Luckily there is only one caller of suc_release(), so we can design
> however dirty interface for convenience, but still it feels wrong to
> design a release() helper that pretends to be usable by general
> public, yet they have to be aware of the fact that some members in
> the struct are still their responsibility to release.  I dunno.

But isn't this fine because that "update_data" member has a "const" on
it:

	/* configuration parameters which are passed on to the children */
	const struct update_data *update_data;

I fix other similar ownership issues you pointed out for a re-roll I'm
preparing, but I don't see why this one is a problem as long as that
"const" is there, it's a slot we use for passing things in externally &
to ferry it along.