Subject: Re: [PATCH v4 07/13] pack-bitmap.c: teach `rev-list --test-bitmap` about incremental MIDXs

From: Taylor Blau <[email-masked]>
Date: Tue, 18 Mar 2025 20:30:05 -0400

On Mon, Mar 17, 2025 at 10:31:06PM -0700, Elijah Newren wrote:
> > +static void bitmap_test_data_prepare(struct bitmap_test_data *tdata,
> > +                                    struct bitmap_index *bitmap_git)
> > +{
> > +       memset(tdata, 0, sizeof(struct bitmap_test_data));
>
> So, the first thing this function does is 0 out tdata.
>
> > +
> > +       tdata->bitmap_git = bitmap_git;
> > +       tdata->base = bitmap_new();
> > +       tdata->commits = ewah_to_bitmap(bitmap_git->commits);
> > +       tdata->trees = ewah_to_bitmap(bitmap_git->trees);
> > +       tdata->blobs = ewah_to_bitmap(bitmap_git->blobs);
> > +       tdata->tags = ewah_to_bitmap(bitmap_git->tags);
> > +
> > +       if (bitmap_git->base) {
> > +               CALLOC_ARRAY(tdata->base_tdata, 1);
>
> We use CALLOC to both allocate the array and set it all to 0...
>
> > +               bitmap_test_data_prepare(tdata->base_tdata, bitmap_git->base);
>
> and then call bitmap_test_data_prepare() which will re-zero it all out.
>
> Should we either ditch the zeroing at the beginning of the function,
> or use xmalloc instead of CALLOC_ARRAY, to avoid duplicate zeroing?

Ah... good point. I think between the two we should drop the
CALLOC_ARRAY() and just xmalloc() it.

Thanks,
Taylor