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