Re: [PATCH 6/6] pack-bitmap.c: factor out `bitmap_index_seek_commit()`

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 21, 2023, 18:16 UTC
Message-ID: <ZBn0iFE5qOJKVeel@nand.local>
In-Reply-To: <20230321181315.GH3119834@coredump.intra.peff.net>

On Tue, Mar 21, 2023 at 02:13:15PM -0400, Jeff King wrote:

> I'm not 100% sure on where these offsets come from. But it looks like
> they're coming from the bitmap lookup table. In which case a bogus value
> there should be an error(), and not a BUG(), I would think.

They do come from the lookup table, yes. I'm not sure that I agree that bogus values here should be an error() or a BUG(), or if I even have a strong preference between one and the other.

But I do think that trying to make it an error() makes it awkward for all of the other callers that want it to be a BUG(), since the detail of whether to call one or the other is private to bitmap_index_seek().

We *could* open-code it, introduce a variant of bitmap_index_seek(), make it take an additional parameter specifying whether to call one over the other, *or* check the bounds ourselves before even calling bitmap_index_seek().

But none of those seem like great options to me, TBH. I would be just as happy to leave this as a BUG(), to be honest.

Thanks, Taylor