In git-restore we need to free the pathspec and pathspec_from_file values from the struct checkout_opts. A simple fix could be to free them in cmd_restore, after the call to checkout_main returns, like we are doing [1][2] in the sibling function cmd_checkout. However, we can do better. We have git-switch and git-restore, both of them spin-offs[3][4] of git-checkout. All three are implemented as thin wrappers around checkout_main. Considering this, it makes a lot of sense to do the cleanup closer to checkout_main. Factor out the call to checkout_main in a function that does both the work and the cleanup, and use it in the three wrappers. As a consequence, mark: t2070, t2071, t2072 and t6418 as leak-free. [1] 9081a421a6 (checkout: fix "branch info" memory leaks, 2021-11-16) [2] 7ce4088ab7 (parse-options: consistently allocate memory in fix_filename(), 2023-03-04) [3] d787d311db (checkout: split part of it to new command 'switch', 2019-03-29) [4] 46e91b663b (checkout: split part of it to new command 'restore', 2019-04-25) --- builtin/checkout.c | 51 +++++++++++++++----------------- t/t2070-restore.sh | 1 + t/t2071-restore-patch.sh | 1 + t/t2072-restore-pathspec-file.sh | 1 + t/t6418-merge-text-auto.sh | 1 + 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 4fe049cf37..2ff4cf88a6 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1702,10 +1702,10 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts, /* create-branch option (either b or c) */ static char cb_option = 'b'; -static int checkout_main(int argc, const char **argv, const char *prefix, - struct checkout_opts *opts, struct option *options, - const char * const usagestr[], - struct branch_info *new_branch_info) +static int checkout_main_1(int argc, const char **argv, const char *prefix, + struct checkout_opts *opts, struct option *options, + const char * const usagestr[], + struct branch_info *new_branch_info) { int parseopt_flags = 0; @@ -1907,6 +1907,20 @@ static int checkout_main(int argc, const char **argv, const char *prefix, return checkout_branch(opts, new_branch_info); } +static int checkout_main(int argc, const char **argv, const char *prefix, + struct checkout_opts *opts, struct option *options, + const char * const usagestr[]) +{ + struct branch_info new_branch_info = { 0 }; + int ret = checkout_main_1(argc, argv, prefix, opts, options, + checkout_usage, &new_branch_info); + branch_info_release(&new_branch_info); + clear_pathspec(&opts->pathspec); + free(opts->pathspec_from_file); + free(options); + return ret; +} + int cmd_checkout(int argc, const char **argv, const char *prefix) { struct checkout_opts opts; @@ -1922,8 +1936,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.dwim_new_local_branch = 1; @@ -1953,13 +1965,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) options = add_common_switch_branch_options(&opts, options); options = add_checkout_path_options(&opts, options); - ret = checkout_main(argc, argv, prefix, &opts, - options, checkout_usage, &new_branch_info); - branch_info_release(&new_branch_info); - clear_pathspec(&opts.pathspec); - free(opts.pathspec_from_file); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + checkout_usage); } int cmd_switch(int argc, const char **argv, const char *prefix) @@ -1977,8 +1984,6 @@ int cmd_switch(int argc, const char **argv, const char *prefix) N_("throw away local modifications")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.dwim_new_local_branch = 1; @@ -1997,11 +2002,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix) cb_option = 'c'; - ret = checkout_main(argc, argv, prefix, &opts, - options, switch_branch_usage, &new_branch_info); - branch_info_release(&new_branch_info); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + switch_branch_usage); } int cmd_restore(int argc, const char **argv, const char *prefix) @@ -2020,8 +2022,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), OPT_END() }; - int ret; - struct branch_info new_branch_info = { 0 }; memset(&opts, 0, sizeof(opts)); opts.accept_ref = 0; @@ -2036,9 +2036,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix) options = add_common_options(&opts, options); options = add_checkout_path_options(&opts, options); - ret = checkout_main(argc, argv, prefix, &opts, - options, restore_usage, &new_branch_info); - branch_info_release(&new_branch_info); - FREE_AND_NULL(options); - return ret; + return checkout_main(argc, argv, prefix, &opts, options, + restore_usage); } diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh index 16d6348b69..ac404945d4 100755 --- a/t/t2070-restore.sh +++ b/t/t2070-restore.sh @@ -5,6 +5,7 @@ test_description='restore basic functionality' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh index 27e85be40a..42d5522119 100755 --- a/t/t2071-restore-patch.sh +++ b/t/t2071-restore-patch.sh @@ -2,6 +2,7 @@ test_description='git restore --patch' +TEST_PASSES_SANITIZE_LEAK=true . ./lib-patch-mode.sh test_expect_success 'setup' ' diff --git a/t/t2072-restore-pathspec-file.sh b/t/t2072-restore-pathspec-file.sh index 8198a1e578..86c9c88788 100755 --- a/t/t2072-restore-pathspec-file.sh +++ b/t/t2072-restore-pathspec-file.sh @@ -2,6 +2,7 @@ test_description='restore --pathspec-from-file' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_tick diff --git a/t/t6418-merge-text-auto.sh b/t/t6418-merge-text-auto.sh index 41288a60ce..48a62cb855 100755 --- a/t/t6418-merge-text-auto.sh +++ b/t/t6418-merge-text-auto.sh @@ -15,6 +15,7 @@ test_description='CRLF merge conflict across text=auto change GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b -- 2.44.0.341.g37b2c3c964