On Fri, Jun 25, 2021 at 01:02:56AM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> 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 :)
Right; since we're looking at the same bit position in each of the
type-level bitmaps, we can't just OR them together, since all of the
bits are in the same place.
And really, the object_type enum doesn't have values that tell us the
type of an object by looking at just a single bit. So
HAS_MULTI_BITS(OBJ_BLOB) would return "true", since OBJ_BLOB is 3.
> > +
> > + 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).
I tend to agree. To restate what you're saying: by the time we get to
the type_name(bitmap_type) we know that bitmap_type is non-zero, so we
assume it's OK to call type_name() on it.
Of course, the object_type_strings does handle the zero argument, so
this is probably a little academic, but good to think through
nonetheless.
Thanks,
Taylor
