Re: [PATCH] Ensure __BYTE_ORDER is always set
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:59:47
Hi, 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.
Makes sense. Sorry I missed this.
In an ideal world I would prefer to just rely on ntohll when it's
decent (meaning that the '#if __BYTE_ORDER != __BIG_ENDIAN' block
could be written as
if (ntohll(1) != 1) {
...
}
or
if (ntohll(1) == 1)
; /* Big endian. Nothing to do.
else {
...
}
). But compat/bswap.h already relies on knowing the endianness at
preprocessing time so that wouldn't buy anything.
Another "in an ideal world" option: make the loop unconditional after
checking that optimizers on big-endian systems realize it's a noop.
In any event, in the real world your patch looks like the right thing
to do.
Reviewed-by: Jonathan Nieder <redacted>