diff options
author | Ævar Arnfjörð Bjarmason <avarab@gmail.com> | 2022-09-01 01:14:23 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2022-09-02 18:18:13 +0200 |
commit | 4c4d3e7c0ae3eda9331227c7cf317afa1d83a0e1 (patch) | |
tree | d381ff80f2f3f4907fcac47391be41f7ca00c0cc /builtin/submodule--helper.c | |
parent | submodule--helper: free some "displaypath" in "struct update_data" (diff) | |
download | git-4c4d3e7c0ae3eda9331227c7cf317afa1d83a0e1.tar.xz git-4c4d3e7c0ae3eda9331227c7cf317afa1d83a0e1.zip |
submodule--helper: free rest of "displaypath" in "struct update_data"
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.
Fix a leak in code added in 51f8f94e5b (submodule--helper: run update
procedures from C, 2021-08-24). We'd always clobber the old
"displaypath" member of the previously passed-in "struct update_data".
A better fix for this would be to remove the "displaypath" member from
the "struct update_data" entirely. Along with "oid", "suboid",
"just_cloned" and "sm_path" it's managing members that mainly need to
be passed between 1-3 stack frames of functions adjacent to this
code. But doing so would be a much larger change (I have it locally,
and fully untangling that in an incremental way is a 10 patch
journey).
So let's go for this much more isolated fix suggested by Glen. We
FREE_AND_NULL() the "update_data->displaypath", the "AND_NULL()" part
of that is needed due to the later "free(ud->displaypath)" in
"update_data_release()" introduced in the preceding commit
Moving ensure_core_worktree() out of update_submodule() may not be
strictly required, but in doing so we are left with the exact same
ordering as before, making this a smaller functional change.
Helped-by: Glen Choo <chooglen@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Reviewed-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to '')
-rw-r--r-- | builtin/submodule--helper.c | 15 |
1 files changed, 8 insertions, 7 deletions
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index cc32eb0dc3..f8470d848e 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2484,13 +2484,6 @@ static int update_submodule(struct update_data *update_data) { int ret; - ret = ensure_core_worktree(update_data->sm_path); - if (ret) - return ret; - - update_data->displaypath = get_submodule_displaypath( - update_data->sm_path, update_data->prefix); - ret = determine_submodule_update_strategy(the_repository, update_data->just_cloned, update_data->sm_path, @@ -2596,7 +2589,15 @@ static int update_submodules(struct update_data *update_data) update_data->just_cloned = ucd.just_cloned; update_data->sm_path = ucd.sub->path; + code = ensure_core_worktree(update_data->sm_path); + if (code) + goto fail; + + update_data->displaypath = get_submodule_displaypath( + update_data->sm_path, update_data->prefix); code = update_submodule(update_data); + FREE_AND_NULL(update_data->displaypath); +fail: if (!code) continue; ret = code; |