Re: [PATCH 3/3] ewah: support platforms that require aligned reads

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Jan 23, 2014, 23:44 UTC
Message-ID: <20140123234456.GF18964@google.com>
In-Reply-To: <20140123212752.GC21705@sigill.intra.peff.net>

Jeff King wrote:

> --- a/ewah/ewah_io.c
> +++ b/ewah/ewah_io.c
> @@ -112,23 +112,38 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)

[...]

> +#if __BYTE_ORDER != __BIG_ENDIAN

Is this portable?

On a platform without __BYTE_ORDER or __BIG_ENDIAN defined, it is interpreted as

	#if 0 != 0

which means that such platforms are assumed to be big endian. Does Mingw define __BYTE_ORDER, for example?

Show 6 quoted lines

> +	{
> +		size_t i;
> +		for (i = 0; i < self->buffer_size; ++i)
> +			self->buffer[i] = ntohll(self->buffer[i]);
> +	}
> +#endif

It's tempting to guard with something like

	if (ntohl(1) != 1) {
		...
	}

The optimizer can tell if this is true or false at compile time, so it shouldn't slow anything down.

With that change,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks for the quick fix.

diff --git i/ewah/ewah_io.c w/ewah/ewah_io.c
index 4a7fae6..5a527a4 100644
--- i/ewah/ewah_io.c
+++ w/ewah/ewah_io.c
@@ -135,13 +135,11 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
 	memcpy(self->buffer, ptr, self->buffer_size * sizeof(uint64_t));
 	ptr += self->buffer_size * sizeof(uint64_t);
 
-#if __BYTE_ORDER != __BIG_ENDIAN
-	{
+	if (ntohl(1) != 1) {
 		size_t i;
 		for (i = 0; i < self->buffer_size; ++i)
 			self->buffer[i] = ntohll(self->buffer[i]);
 	}
-#endif
 
 	self->rlw = self->buffer + get_be32(ptr);