@matheusbsbs / matheus-bagliano / commits / a452d0f4ba

builtin/merge-base: free commit lists

In several functions, we iterate through a commit list by assigning `result = result->next`. As a consequence, we lose the original pointer and eventually leak the list. Rewrite the loops so that we keep the original pointers, then call `free_commit_list()`. Various alternatives were considered: 1) Use `UNLEAK(result)` before the loop. Simple change, but not very pretty. These would definitely be new lows among our usages of UNLEAK. 2) Use `pop_commit()` when looping. Slightly less simple change, but it feels slightly preferable to first display the list, then free it. 3) As in this patch, but with `UNLEAK()` instead of freeing. We'd still go through all the trouble of refactoring the loop, and because it's not super-obvious that we're about to exit, let's just free the lists -- it probably doesn't affect the runtime much. In `handle_independent()` we can drop `result` while we're here and reuse the `revs`-variable instead. That matches several other users of `reduce_heads()`. The memory-leak that this hides will be addressed in the next commit. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed Nov 7, 2017 at 21:39 UTC a452d0f4bae99c9acef6f7db75f6f1d922618732
1 file changed +18 -18
builtin/merge-base.c
+18 -18
@@ -9,20 +9,20 @@
9
10 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
11 {
12 - struct commit_list *result;
12 + struct commit_list *result, *r;
13
14 result = get_merge_bases_many_dirty(rev[0], rev_nr - 1, rev + 1);
15
16 if (!result)
17 return 1;
18
19 - while (result) {
20 - printf("%s\n", oid_to_hex(&result->item->object.oid));
19 + for (r = result; r; r = r->next) {
20 + printf("%s\n", oid_to_hex(&r->item->object.oid));
21 if (!show_all)
22 - return 0;
23 - result = result->next;
22 + break;
23 }
24
25 + free_commit_list(result);
26 return 0;
27 }
28
@@ -51,28 +51,28 @@ static struct commit *get_commit_reference(const char *arg)
51
52 static int handle_independent(int count, const char **args)
53 {
54 - struct commit_list *revs = NULL;
55 - struct commit_list *result;
54 + struct commit_list *revs = NULL, *rev;
55 int i;
56
57 for (i = count - 1; i >= 0; i--)
58 commit_list_insert(get_commit_reference(args[i]), &revs);
59
61 - result = reduce_heads(revs);
62 - if (!result)
60 + revs = reduce_heads(revs);
61 +
62 + if (!revs)
63 return 1;
64
65 - while (result) {
66 - printf("%s\n", oid_to_hex(&result->item->object.oid));
67 - result = result->next;
68 - }
65 + for (rev = revs; rev; rev = rev->next)
66 + printf("%s\n", oid_to_hex(&rev->item->object.oid));
67 +
68 + free_commit_list(revs);
69 return 0;
70 }
71
72 static int handle_octopus(int count, const char **args, int show_all)
73 {
74 struct commit_list *revs = NULL;
75 - struct commit_list *result;
75 + struct commit_list *result, *rev;
76 int i;
77
78 for (i = count - 1; i >= 0; i--)
@@ -83,13 +83,13 @@ static int handle_octopus(int count, const char **args, int show_all)
83 if (!result)
84 return 1;
85
86 - while (result) {
87 - printf("%s\n", oid_to_hex(&result->item->object.oid));
86 + for (rev = result; rev; rev = rev->next) {
87 + printf("%s\n", oid_to_hex(&rev->item->object.oid));
88 if (!show_all)
89 - return 0;
90 - result = result->next;
89 + break;
90 }
91
92 + free_commit_list(result);
93 return 0;
94 }
95