Re: [PATCH v2 01/24] pack-bitmap.c: harden 'test_bitmap_walk()' to check type bitmaps

From: Jeff King <peff@peff.net>
Date: Jul 21, 2021, 09:45 UTC
Message-ID: <YPfsqJrCTFQXhltJ@coredump.intra.peff.net>
In-Reply-To: <a18baeb0b42994ebcb216df5fe69459ba9a33795.1624314293.git.me@ttaylorr.com>

On Mon, Jun 21, 2021 at 06:24:59PM -0400, Taylor Blau wrote:

Show 8 quoted lines

> The special `--test-bitmap` mode of `git rev-list` is used to compare
> the result of an object traversal with a bitmap to check its integrity.
> This mode does not, however, assert that the types of reachable objects
> are stored correctly.
> 
> Harden this mode by teaching it to also check that each time an object's
> bit is marked, the corresponding bit should be set in exactly one of the
> type bitmaps (whose type matches the object's true type).

Yep, makes sense, and the patch looks good.

Show 7 quoted lines

> +{
> +	enum object_type bitmap_type = OBJ_NONE;
> [...]
> +
> +	if (!bitmap_type)
> +		die("object %s not found in type bitmaps",
> +		    oid_to_hex(&obj->oid));

I think the suggestion to do:

  if (bitmap_type == OBJ_NONE)

is reasonable here, as it assumes less about the enum. I do think OBJ_BAD and OBJ_NONE were chosen with these kind of numeric comparisons in mind, but there is no reason to rely on them in places we don't need to.

-Peff