Re: [PATCH 08/19] ewah: compressed bitmap implementation

From: Jeff King <peff@peff.net>
Date: Oct 25, 2013, 03:15 UTC
Message-ID: <20131025031528.GA26283@sigill.intra.peff.net>
In-Reply-To: <xmqqiowmkztz.fsf@gitster.dls.corp.google.com>

On Thu, Oct 24, 2013 at 04:34:48PM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > From: Vicent Marti <tanoku@gmail.com>
> >
> > EWAH is a word-aligned compressed variant of a bitset (i.e. a data
> > structure that acts as a 0-indexed boolean array for many entries).
> 
> I think I've already pointed out some when the original series was
> posted, but this does not seem to compile with -Wdecl-after-stmt
> (possibly other violations may exist; I haven't looked very closely
> yet).

Sorry about that. I typically compile with "-Wall -Werror" which does not catch this, and I didn't carefully go over the ewah code for style, as I considered it mostly a black-box import (though certainly it is worth fixing this case, as it's not just a style issue).

It looks like there is also some void pointer arithmetic...I _thought_ that was handled by -Wall, but I suppose not. Maybe I need to beef up my warning settings.

Note that there is also some use of cast-mmap-file-to-struct, only one instance of which uses __attribute__(packed). However, I notice that the regular pack code is also guilty of this (e.g., see check_packed_git_idx), so I don't know how careful we want to be (and whether we should be using the packed attribute when needed, or if it is sufficiently non-portable that we want to do it by hand in such cases).

-Peff