On Fri, Feb 28, 2025 at 11:01:34AM +0100, Patrick Steinhardt wrote:
> On Tue, Nov 19, 2024 at 05:07:50PM -0500, Taylor Blau wrote:
> > @@ -586,7 +604,29 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_
> > return load_pack_revindex(r, bitmap_git->pack);
> > }
> >
> > -static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git)
> > +static void load_all_type_bitmaps(struct bitmap_index *bitmap_git)
> > +{
> > + struct bitmap_index *curr = bitmap_git;
> > + size_t i = bitmap_git->base_nr - 1;
> > +
> > + ALLOC_ARRAY(bitmap_git->commits_all, bitmap_git->base_nr);
> > + ALLOC_ARRAY(bitmap_git->trees_all, bitmap_git->base_nr);
> > + ALLOC_ARRAY(bitmap_git->blobs_all, bitmap_git->base_nr);
> > + ALLOC_ARRAY(bitmap_git->tags_all, bitmap_git->base_nr);
> > +
> > + while (curr) {
> > + bitmap_git->commits_all[i] = curr->commits;
> > + bitmap_git->trees_all[i] = curr->trees;
> > + bitmap_git->blobs_all[i] = curr->blobs;
> > + bitmap_git->tags_all[i] = curr->tags;
> > +
> > + curr = curr->base;
> > + i -= 1;
>
> Do we want to `BUG()` in case `i == 0` before this statement?
Good idea, but I think we only want to do this when curr is non-NULL
after assigning it to curr->base, so something like:
--- 8< ---
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 48da3f3b5b..7cea838f58 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -616,6 +616,9 @@ static void load_all_type_bitmaps(struct bitmap_index *bitmap_git)
bitmap_git->tags_all[i] = curr->tags;
curr = curr->base;
+ if (curr && !i)
+ BUG("unexpected number of bitmap layers, expected %lu",
+ bitmap_git->base_nr);
i -= 1;
}
}
--- >8 ---
(though note that this is on top of an adjustment I made earlier to make
base_nr 0-indexed as it always should have been, though I don't think it
is relevant to the portion of the change shown here).
Thanks,
Taylor
