object-name: fix leaking commit list items
When calling `get_oid_oneline()`, we pass in a `struct commit_list` that gets modified by the function. This creates a weird situation where the commit list may sometimes be empty after returning, but sometimes it will continue to carry additional commits. In those cases the remainder of the list leaks. Ultimately, the design where we only pass partial ownership to `get_oid_oneline()` feels shoddy. Refactor the code such that we only pass a constant pointer to the list, creating a local copy as needed. Callers are thus always responsible for freeing the commit list, which then allows us to plug a bunch of memory leaks. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Aug 1, 2024 at 12:41 UTC
57fb139b5ee7dcb2ea5182bf33bcd2b07df983c9
3 files changed
+19
-10
object-name.c
+16
-10
@@ -27,7 +27,8 @@
27
#include "date.h"
28
#include "object-file-convert.h"
29
30
-static int get_oid_oneline(struct repository *r, const char *, struct object_id *, struct commit_list *);
30
+static int get_oid_oneline(struct repository *r, const char *, struct object_id *,
31
+ const struct commit_list *);
32
33
typedef int (*disambiguate_hint_fn)(struct repository *, const struct object_id *, void *);
34
@@ -1254,6 +1255,8 @@ static int peel_onion(struct repository *r, const char *name, int len,
1255
prefix = xstrndup(sp + 1, name + len - 1 - (sp + 1));
1256
commit_list_insert((struct commit *)o, &list);
1257
ret = get_oid_oneline(r, prefix, oid, list);
1258
+
1259
+ free_commit_list(list);
1260
free(prefix);
1261
return ret;
1262
}
@@ -1388,9 +1391,10 @@ static int handle_one_ref(const char *path, const struct object_id *oid,
1391
1392
static int get_oid_oneline(struct repository *r,
1393
const char *prefix, struct object_id *oid,
1391
- struct commit_list *list)
1394
+ const struct commit_list *list)
1395
{
1393
- struct commit_list *backup = NULL, *l;
1396
+ struct commit_list *copy = NULL;
1397
+ const struct commit_list *l;
1398
int found = 0;
1399
int negative = 0;
1400
regex_t regex;
@@ -1411,14 +1415,14 @@ static int get_oid_oneline(struct repository *r,
1415
1416
for (l = list; l; l = l->next) {
1417
l->item->object.flags |= ONELINE_SEEN;
1414
- commit_list_insert(l->item, &backup);
1418
+ commit_list_insert(l->item, ©);
1419
}
1416
- while (list) {
1420
+ while (copy) {
1421
const char *p, *buf;
1422
struct commit *commit;
1423
int matches;
1424
1421
- commit = pop_most_recent_commit(&list, ONELINE_SEEN);
1425
+ commit = pop_most_recent_commit(©, ONELINE_SEEN);
1426
if (!parse_object(r, &commit->object.oid))
1427
continue;
1428
buf = repo_get_commit_buffer(r, commit, NULL);
@@ -1433,10 +1437,9 @@ static int get_oid_oneline(struct repository *r,
1437
}
1438
}
1439
regfree(®ex);
1436
- free_commit_list(list);
1437
- for (l = backup; l; l = l->next)
1440
+ for (l = list; l; l = l->next)
1441
clear_commit_marks(l->item, ONELINE_SEEN);
1439
- free_commit_list(backup);
1442
+ free_commit_list(copy);
1443
return found ? 0 : -1;
1444
}
1445
@@ -2024,7 +2027,10 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
2027
refs_for_each_ref(get_main_ref_store(repo), handle_one_ref, &cb);
2028
refs_head_ref(get_main_ref_store(repo), handle_one_ref, &cb);
2029
commit_list_sort_by_date(&list);
2027
- return get_oid_oneline(repo, name + 2, oid, list);
2030
+ ret = get_oid_oneline(repo, name + 2, oid, list);
2031
+
2032
+ free_commit_list(list);
2033
+ return ret;
2034
}
2035
if (namelen < 3 ||
2036
name[2] != ':' ||
t/t1511-rev-parse-caret.sh
+1
@@ -5,6 +5,7 @@ test_description='tests for ref^{stuff}'
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/t6133-pathspec-rev-dwim.sh
+2
@@ -1,6 +1,8 @@
1
#!/bin/sh
2
3
test_description='test dwim of revs versus pathspecs in revision parser'
4
+
5
+TEST_PASSES_SANITIZE_LEAK=true
6
. ./test-lib.sh
7
8
test_expect_success 'setup' '