On Mon, Jun 21 2021, Taylor Blau wrote:
> + enum object_type bitmap_type = OBJ_NONE;
> + int bitmaps_nr = 0;
> +
> + if (bitmap_get(tdata->commits, pos)) {
> + bitmap_type = OBJ_COMMIT;
> + bitmaps_nr++;
> + }
> + if (bitmap_get(tdata->trees, pos)) {
> + bitmap_type = OBJ_TREE;
> + bitmaps_nr++;
> + }
> + if (bitmap_get(tdata->blobs, pos)) {
> + bitmap_type = OBJ_BLOB;
> + bitmaps_nr++;
> + }
> + if (bitmap_get(tdata->tags, pos)) {
> + bitmap_type = OBJ_TAG;
> + bitmaps_nr++;
> + }
This made me wonder if this could be better with something like the
HAS_MULTI_BITS() macro, but that trick probably can't be applied here in
any shape or form :)
> +
> + if (!bitmap_type)
> + die("object %s not found in type bitmaps",
> + oid_to_hex(&obj->oid));
It feels a bit magical to use an enum and then assume to know the enum's
values, I know we do "type < 0" all over the place, but I'd think "if
(bitmap_type == OBJ_NONE)" would be better here....
> +
> + if (bitmaps_nr > 1)
> + die("object %s does not have a unique type",
> + oid_to_hex(&obj->oid));
Or just check the bitmaps_nr instead:
if (!bitmaps_nr)
die("found none");
else if (bitmaps_nr > 1)
...;
Just bikeshedding...
> +
> + if (bitmap_type != obj->type)
> + die("object %s: real type %s, expected: %s",
> + oid_to_hex(&obj->oid),
> + type_name(obj->type),
> + type_name(bitmap_type));
To argue against myself (sort of) about that "== OBJ_NONE" above, if
we're not assuming that then it's sort of weird not to also assume that
type_name(type) won't return a NULL in the case of OBJ_NONE, which it
does (but this code guards against).
