aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPatrick Steinhardt <ps@pks.im>2024-11-25 16:56:25 +0100
committerJunio C Hamano <gitster@pobox.com>2024-11-26 10:22:24 +0900
commit5f9f7fafb7e74ef2965766345f45851732315b00 (patch)
treef8a46571300f69d4c18dc7ba64f03c4c8e9269d8
parentc6c977e82b94a8266a1f24bed6fcddb15bd01d1c (diff)
downloadgit-5f9f7fafb7e74ef2965766345f45851732315b00.tar.gz
bisect: address Coverity warning about potential double free
Coverity has started to warn about a potential double-free in `find_bisection()`. This warning is triggered because we may modify the list head of the passed-in `commit_list` in case it is an UNINTERESTING commit, but still call `free_commit_list()` on the original variable that points to the now-freed head in case where `do_find_bisection()` returns a `NULL` pointer. As far as I can see, this double free cannot happen in practice, as `do_find_bisection()` only returns a `NULL` pointer when it was passed a `NULL` input. So in order to trigger the double free we would have to call `find_bisection()` with a commit list that only consists of UNINTERESTING commits, but I have not been able to construct a case where that happens. Drop the `else` branch entirely as it seems to be a no-op anyway. Another option might be to instead call `free_commit_list()` on `list`, which is the modified version of `commit_list` and thus wouldn't cause a double free. But as mentioned, I couldn't come up with any case where a passed-in non-NULL list becomes empty, so this shouldn't be necessary. And if it ever does become necessary we'd notice anyway via the leak sanitizer. Interestingly enough we did not have a single test exercising this branch: all tests pass just fine even when replacing it with a call to `BUG()`. Add a test that exercises it. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--bisect.c2
-rwxr-xr-xt/t6002-rev-list-bisect.sh5
2 files changed, 5 insertions, 2 deletions
diff --git a/bisect.c b/bisect.c
index f6fa5c235f..d71c4e4b44 100644
--- a/bisect.c
+++ b/bisect.c
@@ -442,8 +442,6 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
best->next = NULL;
}
*reaches = weight(best);
- } else {
- free_commit_list(*commit_list);
}
*commit_list = best;
diff --git a/t/t6002-rev-list-bisect.sh b/t/t6002-rev-list-bisect.sh
index b95a0212ad..daa009c9a1 100755
--- a/t/t6002-rev-list-bisect.sh
+++ b/t/t6002-rev-list-bisect.sh
@@ -308,4 +308,9 @@ test_expect_success '--bisect-all --first-parent' '
test_cmp expect actual
'
+test_expect_success '--bisect without any revisions' '
+ git rev-list --bisect HEAD..HEAD >out &&
+ test_must_be_empty out
+'
+
test_done