Thread (9 messages) flat view 9 messages, 4 authors, 2016-06-15

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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help