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>
Patrick Steinhardt committed
Jun 11, 2024 at 11:20 UTC
3199b22e7d8cf8a95b7fac4e4aaf65638256b226
6 files changed
+12
-12
builtin/am.c
+3
-3
@@ -1573,8 +1573,8 @@ static int build_fake_ancestor(const struct am_state *state, const char *index_f
1573
*/
1574
static int fall_back_threeway(const struct am_state *state, const char *index_path)
1575
{
1576
- struct object_id orig_tree, their_tree, our_tree;
1577
- const struct object_id *bases[1] = { &orig_tree };
1576
+ struct object_id their_tree, our_tree;
1577
+ struct object_id bases[1] = { 0 };
1578
struct merge_options o;
1579
struct commit *result;
1580
char *their_tree_name;
@@ -1588,7 +1588,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
1588
discard_index(the_repository->index);
1589
read_index_from(the_repository->index, index_path, get_git_dir());
1590
1591
- if (write_index_as_tree(&orig_tree, the_repository->index, index_path, 0, NULL))
1591
+ if (write_index_as_tree(&bases[0], the_repository->index, index_path, 0, NULL))
1592
return error(_("Repository lacks necessary blobs to fall back on 3-way merge."));
1593
1594
say(state, stdout, _("Using index info to reconstruct a base tree..."));
builtin/merge-recursive.c
+2
-4
@@ -23,7 +23,7 @@ static char *better_branch_name(const char *branch)
23
24
int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED)
25
{
26
- const struct object_id *bases[21];
26
+ struct object_id bases[21];
27
unsigned bases_count = 0;
28
int i, failed;
29
struct object_id h1, h2;
@@ -49,10 +49,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED)
49
continue;
50
}
51
if (bases_count < ARRAY_SIZE(bases)-1) {
52
- struct object_id *oid = xmalloc(sizeof(struct object_id));
53
- if (repo_get_oid(the_repository, argv[i], oid))
52
+ if (repo_get_oid(the_repository, argv[i], &bases[bases_count++]))
53
die(_("could not parse object '%s'"), argv[i]);
55
- bases[bases_count++] = oid;
54
}
55
else
56
warning(Q_("cannot handle more than %d base. "
merge-recursive.c
+4
-4
@@ -3866,7 +3866,7 @@ int merge_recursive_generic(struct merge_options *opt,
3866
const struct object_id *head,
3867
const struct object_id *merge,
3868
int num_merge_bases,
3869
- const struct object_id **merge_bases,
3869
+ const struct object_id *merge_bases,
3870
struct commit **result)
3871
{
3872
int clean;
@@ -3879,10 +3879,10 @@ int merge_recursive_generic(struct merge_options *opt,
3879
int i;
3880
for (i = 0; i < num_merge_bases; ++i) {
3881
struct commit *base;
3882
- if (!(base = get_ref(opt->repo, merge_bases[i],
3883
- oid_to_hex(merge_bases[i]))))
3882
+ if (!(base = get_ref(opt->repo, &merge_bases[i],
3883
+ oid_to_hex(&merge_bases[i]))))
3884
return err(opt, _("Could not parse object '%s'"),
3885
- oid_to_hex(merge_bases[i]));
3885
+ oid_to_hex(&merge_bases[i]));
3886
commit_list_insert(base, &ca);
3887
}
3888
if (num_merge_bases == 1)
merge-recursive.h
+1
-1
@@ -123,7 +123,7 @@ int merge_recursive_generic(struct merge_options *opt,
123
const struct object_id *head,
124
const struct object_id *merge,
125
int num_merge_bases,
126
- const struct object_id **merge_bases,
126
+ const struct object_id *merge_bases,
127
struct commit **result);
128
129
#endif
t/t6432-merge-recursive-space-options.sh
+1
@@ -14,6 +14,7 @@ test_description='merge-recursive space options
14
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
15
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
16
17
+TEST_PASSES_SANITIZE_LEAK=true
18
. ./test-lib.sh
19
20
test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
t/t6434-merge-recursive-rename-options.sh
+1
@@ -29,6 +29,7 @@ mentions this in a different context).
29
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
30
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
31
32
+TEST_PASSES_SANITIZE_LEAK=true
33
. ./test-lib.sh
34
35
get_expected_stages () {