@matheusbsbs / matheus-bagliano / commits / f92dbdbc6a

revisions API: don't leak memory on argv elements that need free()-ing

Add a "free_removed_argv_elements" member to "struct setup_revision_opt", and use it to fix several memory leaks. We have various memory leaks in APIs that take and munge "const char **argv", e.g. parse_options(). Sometimes these APIs are given the "argv" we get to the "main" function, in which case we don't leak memory, but other times we're giving it the "v" member of a "struct strvec" we created. There's several potential ways to fix those sort of leaks, we could add a "nodup" mode to "struct strvec", which would work for the cases where we push constant strings to it. But that wouldn't work as soon as we used strvec_pushf(), or otherwise needed to duplicate or create a string for that "struct strvec". Let's instead make it the responsibility of the revisions API. If it's going to clobber elements of argv it can also free() them, which it will now do if instructed to do so via "free_removed_argv_elements". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Aug 2, 2022 at 17:33 UTC f92dbdbc6a8aa5f3979f4bb7a7b9bbc8ec9b4aa6

6 files changed +17 -5

bisect.c

+4 -2

@@ -653,6 +653,9 @@ static void bisect_rev_setup(struct repository *r, struct rev_info *revs,

653 const char *bad_format, const char *good_format,

654 int read_paths)

655 {

656 + struct setup_revision_opt opt = {

657 + .free_removed_argv_elements = 1,

658 + };

659 int i;

660

661 repo_init_revisions(r, revs, prefix);

@@ -669,8 +672,7 @@ static void bisect_rev_setup(struct repository *r, struct rev_info *revs,

672 if (read_paths)

673 read_bisect_paths(rev_argv);

674

672 - setup_revisions(rev_argv->nr, rev_argv->v, revs, NULL);

673 - /* XXX leak rev_argv, as "revs" may still be pointing to it */

675 + setup_revisions(rev_argv->nr, rev_argv->v, revs, &opt);

676 }

677

678 static void bisect_common(struct rev_info *revs)

builtin/submodule--helper.c

+4 -1

@@ -1104,6 +1104,9 @@ static int compute_summary_module_list(struct object_id *head_oid,

1104 {

1105 struct strvec diff_args = STRVEC_INIT;

1106 struct rev_info rev;

1107 + struct setup_revision_opt opt = {

1108 + .free_removed_argv_elements = 1,

1109 + };

1110 struct module_cb_list list = MODULE_CB_LIST_INIT;

1111 int ret = 0;

1112

@@ -1121,7 +1124,7 @@ static int compute_summary_module_list(struct object_id *head_oid,

1124 init_revisions(&rev, info->prefix);

1125 rev.abbrev = 0;

1126 precompose_argv_prefix(diff_args.nr, diff_args.v, NULL);

1124 - setup_revisions(diff_args.nr, diff_args.v, &rev, NULL);

1127 + setup_revisions(diff_args.nr, diff_args.v, &rev, &opt);

1128 rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK;

1129 rev.diffopt.format_callback = submodule_summary_callback;

1130 rev.diffopt.format_callback_data = &list;

remote.c

+4 -1

@@ -2169,6 +2169,9 @@ static int stat_branch_pair(const char *branch_name, const char *base,

2169 struct object_id oid;

2170 struct commit *ours, *theirs;

2171 struct rev_info revs;

2172 + struct setup_revision_opt opt = {

2173 + .free_removed_argv_elements = 1,

2174 + };

2175 struct strvec argv = STRVEC_INIT;

2176

2177 /* Cannot stat if what we used to build on no longer exists */

@@ -2203,7 +2206,7 @@ static int stat_branch_pair(const char *branch_name, const char *base,

2206 strvec_push(&argv, "--");

2207

2208 repo_init_revisions(the_repository, &revs, NULL);

2206 - setup_revisions(argv.nr, argv.v, &revs, NULL);

2209 + setup_revisions(argv.nr, argv.v, &revs, &opt);

2210 if (prepare_revision_walk(&revs))

2211 die(_("revision walk setup failed"));

2212

revision.c

@@ -2784,6 +2784,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s

2784 const char *arg = argv[i];

2785 if (strcmp(arg, "--"))

2786 continue;

2787 + if (opt && opt->free_removed_argv_elements)

2788 + free((char *)argv[i]);

2789 argv[i] = NULL;

2790 argc = i;

2791 if (argv[i + 1])

revision.h

+2 -1

@@ -375,7 +375,8 @@ struct setup_revision_opt {

375 const char *def;

376 void (*tweak)(struct rev_info *, struct setup_revision_opt *);

377 unsigned int assume_dashdash:1,

378 - allow_exclude_promisor_objects:1;

378 + allow_exclude_promisor_objects:1,

379 + free_removed_argv_elements:1;

380 unsigned revarg_opt;

381 };

382 int setup_revisions(int argc, const char **argv, struct rev_info *revs,

t/t2020-checkout-detach.sh

@@ -4,6 +4,7 @@ test_description='checkout into detached HEAD state'

4 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main

5 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME

7 +TEST_PASSES_SANITIZE_LEAK=true

8 . ./test-lib.sh

10 check_detached () {