Re: [PATCH v3 24/26] submodule--helper: free rest of "displaypath" in "struct update_data"
From: Glen Choo <hidden>
Date: 2022-07-26 01:06:43
Ævar Arnfjörð Bjarmason [off-list ref] writes:
quoted hunk ↗ jump to hunk
Fix a leak in code added in c51f8f94e5b (submodule--helper: run update procedures from C, 2021-08-24), we clobber the "displaypath" member of the passed-in "struct update_data" both so that die() messages in this update_submodule() function itself can use it, and for the run_update_procedure() called within this function. Signed-off-by: Ævar Arnfjörð Bjarmason <redacted> --- builtin/submodule--helper.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b65665105e7..4e70a74357c 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c@@ -2551,10 +2551,11 @@ static int update_submodule(struct update_data *update_data, int *must_die_on_failure) { int ret = 1; + char *to_free, *restore = update_data->displaypath; ensure_core_worktree(update_data->sm_path); - update_data->displaypath = get_submodule_displaypath( + update_data->displaypath = to_free = get_submodule_displaypath( update_data->sm_path, update_data->prefix); determine_submodule_update_strategy(the_repository, update_data->just_cloned,@@ -2628,6 +2629,9 @@ static int update_submodule(struct update_data *update_data, ret = 0; cleanup: + free(to_free); + update_data->displaypath = restore; + return ret; }
I'm not sure why we need to have "restore". We set "update_data->displaypath" so that we can use it inside this function (and its call chain), but we don't care about it outside of this call chain at all. If the goal is to avoid exposing a free()-d pointer, could we just do FREE_AND_NULL(update_data->displaypath); instead?
-- 2.37.1.1095.g0bd6f54ba8a