On Fri, Mar 14, 2025 at 1:18 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
>
> Implement support for the special `--test-bitmap` mode of `git rev-list`
> when using incremental MIDXs.
>
> The bitmap_test_data structure is extended to contain a "base" pointer
> that mirrors the structure of the bitmap chain that it is being used to
> test.
>
> When we find a commit to test, we first chase down the ->base pointer to
> find the appropriate bitmap_test_data for the bitmap layer that the
> given commit is contained within, and then perform the test on that
> bitmap.
>
> In order to implement this, light modifications are made to
> bitmap_for_commit() to reimplement it in terms of a new function,
> find_bitmap_for_commit(), which fills out a pointer which indicates the
> bitmap layer which contains the given commit.
>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> pack-bitmap.c | 107 ++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 86 insertions(+), 21 deletions(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index 7a41535425..bb09ce3cf5 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
[...]
> +
> +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?
> + }
> +}
Didn't spot anything else.
