diff options
author | Patrick Steinhardt <ps@pks.im> | 2024-06-11 11:20:09 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2024-06-11 22:15:06 +0200 |
commit | 3199b22e7d8cf8a95b7fac4e4aaf65638256b226 (patch) | |
tree | 07979b04cb6473442278afad4a84aacb60ee4b1a | |
parent | builtin/difftool: plug memory leaks in `run_dir_diff()` (diff) | |
download | git-3199b22e7d8cf8a95b7fac4e4aaf65638256b226.tar.xz git-3199b22e7d8cf8a95b7fac4e4aaf65638256b226.zip |
builtin/merge-recursive: fix leaking object ID bases
In `cmd_merge_recursive()` we have a static array of object ID bases
that we pass to `merge_recursive_generic()`. This interface is somewhat
weird though because the latter function accepts a pointer to a pointer
of object IDs, which requires us to allocate the object IDs on the heap.
And as we never free those object IDs, the end result is a leak.
While we can easily solve this leak by just freeing the respective
object IDs, the whole calling convention is somewhat weird. Instead,
refactor `merge_recursive_generic()` to accept a plain pointer to object
IDs so that we can avoid allocating them altogether.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | builtin/am.c | 6 | ||||
-rw-r--r-- | builtin/merge-recursive.c | 6 | ||||
-rw-r--r-- | merge-recursive.c | 8 | ||||
-rw-r--r-- | merge-recursive.h | 2 | ||||
-rwxr-xr-x | t/t6432-merge-recursive-space-options.sh | 1 | ||||
-rwxr-xr-x | t/t6434-merge-recursive-rename-options.sh | 1 |
6 files changed, 12 insertions, 12 deletions
diff --git a/builtin/am.c b/builtin/am.c index 36839029d2..4ba44e2d70 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1573,8 +1573,8 @@ static int build_fake_ancestor(const struct am_state *state, const char *index_f */ static int fall_back_threeway(const struct am_state *state, const char *index_path) { - struct object_id orig_tree, their_tree, our_tree; - const struct object_id *bases[1] = { &orig_tree }; + struct object_id their_tree, our_tree; + struct object_id bases[1] = { 0 }; struct merge_options o; struct commit *result; char *their_tree_name; @@ -1588,7 +1588,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa discard_index(the_repository->index); read_index_from(the_repository->index, index_path, get_git_dir()); - if (write_index_as_tree(&orig_tree, the_repository->index, index_path, 0, NULL)) + if (write_index_as_tree(&bases[0], the_repository->index, index_path, 0, NULL)) return error(_("Repository lacks necessary blobs to fall back on 3-way merge.")); say(state, stdout, _("Using index info to reconstruct a base tree...")); diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c index c2ce044a20..82bebea15b 100644 --- a/builtin/merge-recursive.c +++ b/builtin/merge-recursive.c @@ -23,7 +23,7 @@ static char *better_branch_name(const char *branch) int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED) { - const struct object_id *bases[21]; + struct object_id bases[21]; unsigned bases_count = 0; int i, failed; struct object_id h1, h2; @@ -49,10 +49,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED) continue; } if (bases_count < ARRAY_SIZE(bases)-1) { - struct object_id *oid = xmalloc(sizeof(struct object_id)); - if (repo_get_oid(the_repository, argv[i], oid)) + if (repo_get_oid(the_repository, argv[i], &bases[bases_count++])) die(_("could not parse object '%s'"), argv[i]); - bases[bases_count++] = oid; } else warning(Q_("cannot handle more than %d base. " diff --git a/merge-recursive.c b/merge-recursive.c index 8c8e8b4868..eff73dac02 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -3866,7 +3866,7 @@ int merge_recursive_generic(struct merge_options *opt, const struct object_id *head, const struct object_id *merge, int num_merge_bases, - const struct object_id **merge_bases, + const struct object_id *merge_bases, struct commit **result) { int clean; @@ -3879,10 +3879,10 @@ int merge_recursive_generic(struct merge_options *opt, int i; for (i = 0; i < num_merge_bases; ++i) { struct commit *base; - if (!(base = get_ref(opt->repo, merge_bases[i], - oid_to_hex(merge_bases[i])))) + if (!(base = get_ref(opt->repo, &merge_bases[i], + oid_to_hex(&merge_bases[i])))) return err(opt, _("Could not parse object '%s'"), - oid_to_hex(merge_bases[i])); + oid_to_hex(&merge_bases[i])); commit_list_insert(base, &ca); } if (num_merge_bases == 1) diff --git a/merge-recursive.h b/merge-recursive.h index e67d38c303..839eb6436e 100644 --- a/merge-recursive.h +++ b/merge-recursive.h @@ -123,7 +123,7 @@ int merge_recursive_generic(struct merge_options *opt, const struct object_id *head, const struct object_id *merge, int num_merge_bases, - const struct object_id **merge_bases, + const struct object_id *merge_bases, struct commit **result); #endif diff --git a/t/t6432-merge-recursive-space-options.sh b/t/t6432-merge-recursive-space-options.sh index db4b77e63d..c93538b0c3 100755 --- a/t/t6432-merge-recursive-space-options.sh +++ b/t/t6432-merge-recursive-space-options.sh @@ -14,6 +14,7 @@ test_description='merge-recursive space options 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 diff --git a/t/t6434-merge-recursive-rename-options.sh b/t/t6434-merge-recursive-rename-options.sh index a11707835b..df1d0c156c 100755 --- a/t/t6434-merge-recursive-rename-options.sh +++ b/t/t6434-merge-recursive-rename-options.sh @@ -29,6 +29,7 @@ mentions this in a different context). GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh get_expected_stages () { |