Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.
