Re: [BUG] add_again() off-by-one error in custom format

From: Jeff King <peff@peff.net>
Date: Jun 22, 2017, 23:15 UTC
Message-ID: <20170622231506.prfndno7cipp2d7f@sigill.intra.peff.net>
In-Reply-To: <87848e2d-9766-7dad-62cf-ded0c18f6dc3@web.de>

On Thu, Jun 22, 2017 at 08:19:40PM +0200, René Scharfe wrote:

Show 13 quoted lines

> > I'd be OK with keeping it if we could reduce the number of magic
> > numbers. E.g,. rather than 32 elsewhere use:
> > 
> >    (sizeof(*loose_objects_subdir_bitmap) * CHAR_BIT)
> 
> We have a bitsizeof macro for that.
> 
> > and similarly rather than 8 here use
> > 
> >    256 / sizeof(*loose_objects_subdir_bitmap) / CHAR_BIT
> 
> If we're pretending not to know the number of bits in a byte then we
> need to round up, and we have DIV_ROUND_UP for that. :)

Thanks, I meant to mention the rounding thing but forgot. I didn't know about either of those macros, though.

Show 7 quoted lines

> There is another example of a bitmap in shallow_c (just search for
> "32").  It would benefit from the macros mentioned above.  That
> might make it easier to switch to the native word size (unsigned int)
> instead of using uint32_t everywhere.
> 
> But perhaps this one is actually a candidate for using EWAH, depending
> on the number of refs the code is supposed to handle.

I thought at first you meant real EWAH bitmaps. But those aren't random access (so you have to read them into an uncompressed bitmap in memory, which is why ewah/bitmap.c exists). But if you just mean reusing those bitmaps, then yeah, possibly.

It looks like it's using a commit slab, though. If you know the number of bits you need up front, then the commit slab can do it without any waste. I didn't dig enough to see if that's what it's doing or not.

-Peff