Re: [PATCH v2 4/6] pack-bitmap: prepare to read lookup table extension

From: Abhradeep Chakraborty <chakrabortyabhradeep79@gmail.com>
Date: Jun 30, 2022, 08:35 UTC
Message-ID: <20220630083517.6411-1-chakrabortyabhradeep79@gmail.com>
In-Reply-To: <Yry4ElVFQEsVbqse@nand.local>

Taylor Blau <me@ttaylorr.com> wrote:

Show 14 quoted lines

> I think having some indication (maybe via a trace2 region?) that we're
> actually executing this code would be good. Although it's going to be
> *really* noisy, so probably not a good idea to do that in general.
>
> Stolee runs some coverage tests that show lines that we aren't
> exercising via tests. So making sure that this doesn't show up in that
> report when you run it locally would be good.
>
> See some information from him about how to run those tests locally here:
>
>     https://lore.kernel.org/git/00a57a1d-0566-8f54-26b2-0f3558bde88d@github.com/
>
> (TL;DR: run `make coverage-test` and make sure that these lines don't
> show up ;-)).

Got it. Thanks.

Show 6 quoted lines

> If we had something like that, we could call that function from within
> `test_bitmap_commits()` before reading the keys and values out of
> `bitmap_git->bitmaps`.
>
> An alternative approach would be to read the table directly when it
> exists, perhaps something like this:

I think we have a simpler fix than what you suggested here. What if We do it like this way -

    if (bitmap_git->table_lookup) {
	if (load_bitmap_entries_v1(bitmap_git) < 0)
	    die(_("failed to load bitmap indexes"));
    }

Is this okay for you?