From a452d0f4bae99c9acef6f7db75f6f1d922618732 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Tue, 7 Nov 2017 21:39:44 +0100 Subject: builtin/merge-base: free commit lists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- builtin/merge-base.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'builtin/merge-base.c') diff --git a/builtin/merge-base.c b/builtin/merge-base.c index 6dbd167d3b..e17835fabb 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -9,20 +9,20 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all) { - struct commit_list *result; + struct commit_list *result, *r; result = get_merge_bases_many_dirty(rev[0], rev_nr - 1, rev + 1); if (!result) return 1; - while (result) { - printf("%s\n", oid_to_hex(&result->item->object.oid)); + for (r = result; r; r = r->next) { + printf("%s\n", oid_to_hex(&r->item->object.oid)); if (!show_all) - return 0; - result = result->next; + break; } + free_commit_list(result); return 0; } @@ -51,28 +51,28 @@ static struct commit *get_commit_reference(const char *arg) static int handle_independent(int count, const char **args) { - struct commit_list *revs = NULL; - struct commit_list *result; + struct commit_list *revs = NULL, *rev; int i; for (i = count - 1; i >= 0; i--) commit_list_insert(get_commit_reference(args[i]), &revs); - result = reduce_heads(revs); - if (!result) + revs = reduce_heads(revs); + + if (!revs) return 1; - while (result) { - printf("%s\n", oid_to_hex(&result->item->object.oid)); - result = result->next; - } + for (rev = revs; rev; rev = rev->next) + printf("%s\n", oid_to_hex(&rev->item->object.oid)); + + free_commit_list(revs); return 0; } static int handle_octopus(int count, const char **args, int show_all) { struct commit_list *revs = NULL; - struct commit_list *result; + struct commit_list *result, *rev; int i; for (i = count - 1; i >= 0; i--) @@ -83,13 +83,13 @@ static int handle_octopus(int count, const char **args, int show_all) if (!result) return 1; - while (result) { - printf("%s\n", oid_to_hex(&result->item->object.oid)); + for (rev = result; rev; rev = rev->next) { + printf("%s\n", oid_to_hex(&rev->item->object.oid)); if (!show_all) - return 0; - result = result->next; + break; } + free_commit_list(result); return 0; } -- cgit v1.2.1 From 4da72644b768b0491110a8ba0aa84d32b6bde41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C3=85gren?= Date: Tue, 7 Nov 2017 21:39:45 +0100 Subject: reduce_heads: fix memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We currently have seven callers of `reduce_heads(foo)`. Six of them do not use the original list `foo` again, and actually, all six of those end up leaking it. Introduce and use `reduce_heads_replace(&foo)` as a leak-free version of `foo = reduce_heads(foo)` to fix several of these. Fix the remaining leaks using `free_commit_list()`. While we're here, document `reduce_heads()` and mark it as `extern`. Signed-off-by: Martin Ågren Signed-off-by: Junio C Hamano --- builtin/merge-base.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'builtin/merge-base.c') diff --git a/builtin/merge-base.c b/builtin/merge-base.c index e17835fabb..24f6c71935 100644 --- a/builtin/merge-base.c +++ b/builtin/merge-base.c @@ -57,7 +57,7 @@ static int handle_independent(int count, const char **args) for (i = count - 1; i >= 0; i--) commit_list_insert(get_commit_reference(args[i]), &revs); - revs = reduce_heads(revs); + reduce_heads_replace(&revs); if (!revs) return 1; @@ -78,7 +78,9 @@ static int handle_octopus(int count, const char **args, int show_all) for (i = count - 1; i >= 0; i--) commit_list_insert(get_commit_reference(args[i]), &revs); - result = reduce_heads(get_octopus_merge_bases(revs)); + result = get_octopus_merge_bases(revs); + free_commit_list(revs); + reduce_heads_replace(&result); if (!result) return 1; -- cgit v1.2.1