Re: [PATCH] Ensure __BYTE_ORDER is always set

[Jeff King <peff@peff.net>]

On Thu, Jan 30, 2014 at 02:55:41PM -0500, Brian Gernhardt wrote:

> a201c20 (ewah: support platforms that require aligned reads) added a
> reliance on the existence of __BYTE_ORDER and __BIG_ENDIAN.  However,
> these macros are spelled without the leading __ on some platforms (OS
> X at least).  In this case, the endian-swapping code was added even
> when unnecessary, which caused assertion failures in
> t5310-pack-bitmaps.sh as the code that used the bitmap would read past
> the end.
> 
> We already had code to handle this case in compat/bswap.h, but it was
> only used if we couldn't already find a reasonable version of bswap64.
> Move the macro-defining and checking code out of a conditional so that
> either __BYTE_ORDER is defined or we get a compilation error instead
> of a runtime error in the bitmap code.

Thanks, this makes sense, and matches the assumption that a201c20 made.

I do find the failure mode interesting. The endian-swapping code kicked
in when it did not, meaning your are on a big-endian system. Is this on
an ancient PPC Mac? Or is the problem that the code did not kick in when
it should?

Either way, we should perhaps be more careful in the bitmap code, too,
that the values we get are sensible. It's better to die("your bitmap is
broken") than to read off the end of the array. I can't seem to trigger
the same failure mode, though. On my x86 system, turning off the
endian-swap (i.e., the opposite of what should happen) makes t5310 fail,
but it is because we end up trying to set the bit very far into a
dynamic bitfield, and die allocating memory.

-Peff