Re: [PATCH] test_bitmap_hashes(): handle repository without bitmaps
From: Junio C Hamano <hidden>
Date: 2021-11-05 18:52:20
Jeff King [off-list ref] writes:
If prepare_bitmap_git() returns NULL (one easy-to-trigger cause being that the repository does not have bitmaps at all), then we'll segfault accessing bitmap_git->hashes: $ t/helper/test-tool bitmap dump-hashes Segmentation fault We should treat this the same as a repository with bitmaps but no name-hashes, and quietly produce an empty output. The later call to free_bitmap_index() in the cleanup label is OK, as it treats a NULL pointer as a noop. This isn't a big deal in practice, as this function is intended for and used only by test-tool. It's probably worth fixing to avoid confusion, but not worth adding coverage for this to the test suite. Signed-off-by: Jeff King <redacted> --- This is new in the v2.34.0 cycle, but it's so low impact it doesn't matter much if we ship with the bug. OTOH, it's pretty low-risk since it is only run by the test suite.
;-) I wonder how you found it. Diagnosing a repository that did not seem healthy? What I am getting at is if we want a new option to make a plumbing command, other than the test-tool, that calls this function, as the latter is usually not deployed in the field. Will queue. Thanks.
quoted hunk
pack-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/pack-bitmap.c b/pack-bitmap.c index a56ceb9441..f772d3cb7f 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c@@ -1759,7 +1759,7 @@ int test_bitmap_hashes(struct repository *r) struct object_id oid; uint32_t i, index_pos; - if (!bitmap_git->hashes) + if (!bitmap_git || !bitmap_git->hashes) goto cleanup; for (i = 0; i < bitmap_num_objects(bitmap_git); i++) {