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 <jrnieder@xxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html