From 2ec8823de3bd0ca8253ddbd07f58b66afcfabe96 Mon Sep 17 00:00:00 2001 From: Yasushi SHOJI Date: Mon, 8 Jan 2018 22:35:12 +0900 Subject: [PATCH 2/2] bisect: handle empty list in best_bisection_sorted() In 7c117184d7 ("bisect: fix off-by-one error in `best_bisection_sorted()`", 2017-11-05) the more careful logic dealing with freeing p->next in 50e62a8e70 ("rev-list: implement --bisect-all", 2007-10-22) was removed. This function, and also all other functions below find_bisection() to be complete, will be called with an empty commit list if all commits are _uninteresting_. So the functions must be prepared for it. This commit, instead of restoring the check, moves the clean-up code into the loop. To do that this commit changes the non-last-case branch in the loop to a last-case branch, and clean-up unused trailing elements there. We could, on the other hand, do a big "if list" condition at the very beginning. But, that doesn't sound right for the current code base. No other functions does that but all blocks in the functions are tolerant to an empty list. By the way, as you can tell from the non-last-case branch we had in the loop, this fix shouldn't cause a pipeline stall on every iteration on modern processors with a branch predictor. Signed-off-by: Yasushi ShOJI --- bisect.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bisect.c b/bisect.c index fb3e44903..cec4a537f 100644 --- a/bisect.c +++ b/bisect.c @@ -218,7 +218,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n cnt++; } QSORT(array, cnt, compare_commit_dist); - for (p = list, i = 0; i < cnt; i++) { + for (p = list, i = 0; i < cnt; i++, p = p->next) { struct object *obj = &(array[i].commit->object); strbuf_reset(&buf); @@ -226,11 +226,13 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n add_name_decoration(DECORATION_NONE, buf.buf, obj); p->item = array[i].commit; - if (i < cnt - 1) - p = p->next; + + if (i == cnt) { + /* clean-up unused elements if any */ + free_commit_list(p->next); + p->next = NULL; + } } - free_commit_list(p->next); - p->next = NULL; strbuf_release(&buf); free(array); return list; -- 2.15.1