checkout: plug some leaks in git-restore
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 even 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. Move the cleanups, including the new_branch_info variable, to checkout_main. 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@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Rubén Justo committed
Mar 14, 2024 at 19:08 UTC
2f64da0790900f9c83f697730047282b7d22f5b5
5 files changed
+25
-30
builtin/checkout.c
+21
-30
@@ -1687,10 +1687,11 @@ static char cb_option = 'b';
1687
1688
static int checkout_main(int argc, const char **argv, const char *prefix,
1689
struct checkout_opts *opts, struct option *options,
1690
- const char * const usagestr[],
1691
- struct branch_info *new_branch_info)
1690
+ const char * const usagestr[])
1691
{
1692
int parseopt_flags = 0;
1693
+ struct branch_info new_branch_info = { 0 };
1694
+ int ret;
1695
1696
opts->overwrite_ignore = 1;
1697
opts->prefix = prefix;
@@ -1806,7 +1807,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
1807
opts->track == BRANCH_TRACK_UNSPECIFIED &&
1808
!opts->new_branch;
1809
int n = parse_branchname_arg(argc, argv, dwim_ok,
1809
- new_branch_info, opts, &rev);
1810
+ &new_branch_info, opts, &rev);
1811
argv += n;
1812
argc -= n;
1813
} else if (!opts->accept_ref && opts->from_treeish) {
@@ -1815,7 +1816,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
1816
if (repo_get_oid_mb(the_repository, opts->from_treeish, &rev))
1817
die(_("could not resolve %s"), opts->from_treeish);
1818
1818
- setup_new_branch_info_and_source_tree(new_branch_info,
1819
+ setup_new_branch_info_and_source_tree(&new_branch_info,
1820
opts, &rev,
1821
opts->from_treeish);
1822
@@ -1835,7 +1836,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
1836
* Try to give more helpful suggestion.
1837
* new_branch && argc > 1 will be caught later.
1838
*/
1838
- if (opts->new_branch && argc == 1 && !new_branch_info->commit)
1839
+ if (opts->new_branch && argc == 1 && !new_branch_info.commit)
1840
die(_("'%s' is not a commit and a branch '%s' cannot be created from it"),
1841
argv[0], opts->new_branch);
1842
@@ -1885,9 +1886,16 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
1886
}
1887
1888
if (opts->patch_mode || opts->pathspec.nr)
1888
- return checkout_paths(opts, new_branch_info);
1889
+ ret = checkout_paths(opts, &new_branch_info);
1890
else
1890
- return checkout_branch(opts, new_branch_info);
1891
+ ret = checkout_branch(opts, &new_branch_info);
1892
+
1893
+ branch_info_release(&new_branch_info);
1894
+ clear_pathspec(&opts->pathspec);
1895
+ free(opts->pathspec_from_file);
1896
+ free(options);
1897
+
1898
+ return ret;
1899
}
1900
1901
int cmd_checkout(int argc, const char **argv, const char *prefix)
@@ -1905,8 +1913,6 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1913
OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")),
1914
OPT_END()
1915
};
1908
- int ret;
1909
- struct branch_info new_branch_info = { 0 };
1916
1917
memset(&opts, 0, sizeof(opts));
1918
opts.dwim_new_local_branch = 1;
@@ -1936,13 +1942,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
1942
options = add_common_switch_branch_options(&opts, options);
1943
options = add_checkout_path_options(&opts, options);
1944
1939
- ret = checkout_main(argc, argv, prefix, &opts,
1940
- options, checkout_usage, &new_branch_info);
1941
- branch_info_release(&new_branch_info);
1942
- clear_pathspec(&opts.pathspec);
1943
- free(opts.pathspec_from_file);
1944
- FREE_AND_NULL(options);
1945
- return ret;
1945
+ return checkout_main(argc, argv, prefix, &opts, options,
1946
+ checkout_usage);
1947
}
1948
1949
int cmd_switch(int argc, const char **argv, const char *prefix)
@@ -1960,8 +1961,6 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
1961
N_("throw away local modifications")),
1962
OPT_END()
1963
};
1963
- int ret;
1964
- struct branch_info new_branch_info = { 0 };
1964
1965
memset(&opts, 0, sizeof(opts));
1966
opts.dwim_new_local_branch = 1;
@@ -1980,11 +1979,8 @@ int cmd_switch(int argc, const char **argv, const char *prefix)
1979
1980
cb_option = 'c';
1981
1983
- ret = checkout_main(argc, argv, prefix, &opts,
1984
- options, switch_branch_usage, &new_branch_info);
1985
- branch_info_release(&new_branch_info);
1986
- FREE_AND_NULL(options);
1987
- return ret;
1982
+ return checkout_main(argc, argv, prefix, &opts, options,
1983
+ switch_branch_usage);
1984
}
1985
1986
int cmd_restore(int argc, const char **argv, const char *prefix)
@@ -2003,8 +1999,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
1999
OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")),
2000
OPT_END()
2001
};
2006
- int ret;
2007
- struct branch_info new_branch_info = { 0 };
2002
2003
memset(&opts, 0, sizeof(opts));
2004
opts.accept_ref = 0;
@@ -2019,9 +2013,6 @@ int cmd_restore(int argc, const char **argv, const char *prefix)
2013
options = add_common_options(&opts, options);
2014
options = add_checkout_path_options(&opts, options);
2015
2022
- ret = checkout_main(argc, argv, prefix, &opts,
2023
- options, restore_usage, &new_branch_info);
2024
- branch_info_release(&new_branch_info);
2025
- FREE_AND_NULL(options);
2026
- return ret;
2016
+ return checkout_main(argc, argv, prefix, &opts, options,
2017
+ restore_usage);
2018
}
t/t2070-restore.sh
+1
@@ -5,6 +5,7 @@ test_description='restore basic functionality'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
11
test_expect_success 'setup' '
t/t2071-restore-patch.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='git restore --patch'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./lib-patch-mode.sh
7
8
test_expect_success PERL 'setup' '
t/t2072-restore-pathspec-file.sh
+1
@@ -2,6 +2,7 @@
2
3
test_description='restore --pathspec-from-file'
4
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_tick
t/t6418-merge-text-auto.sh
+1
@@ -15,6 +15,7 @@ test_description='CRLF merge conflict across text=auto change
15
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
16
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
17
18
+TEST_PASSES_SANITIZE_LEAK=true
19
. ./test-lib.sh
20
21
test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b