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)
Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
---
Range-diff:
1: d54a2c4dcc ! 1: 78ad33fa02 checkout: plug some leaks in git-restore
@@ Commit message
[4] 46e91b663b (checkout: split part of it to new command 'restore',
2019-04-25)
+ Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
+
## builtin/checkout.c ##
@@ builtin/checkout.c: static struct option *add_checkout_path_options(struct checkout_opts *opts,
/* create-branch option (either b or c) */
Thanks Eric.
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.43.0
