Re: [PATCH v8 01/12] bswap: add 64 bit endianness helper get_be64

On Sat, Sep 23, 2017 at 11:31:50PM +0000, Ben Peart wrote:

quoted

diff --git a/compat/bswap.h b/compat/bswap.h index 6b22c4621..9dc79bdf5

100644

--- a/compat/bswap.h
+++ b/compat/bswap.h

@@ -183,8 +183,8 @@ static inline uint32_t get_be32(const void *ptr)  static

inline uint64_t get_be64(const void *ptr)  {
 	const unsigned char *p = ptr;
-	return	(uint64_t)get_be32(p[0]) << 32 |
-		(uint64_t)get_be32(p[4]) <<  0;
+	return	(uint64_t)get_be32(p + 0) << 32 |
+		(uint64_t)get_be32(p + 4) <<  0;

This is surprising.  Every other function in the file uses the p[x] syntax.  Just for
consistency, is there a way to stick to that syntax but still make it work correctly?
Is there a typecast that can make it work?

The other ones are accessing the byte values directly, but since you are
building on get_be32 here, you have to pass the pointer.  So:

  return (uint64_t)get_be32(&p[0]) << 32 |
         (uint64_t)get_be32(&p[4]) <<  0;

would work.  Or of course you could just spell it out like the others:

  return (uint64_t)p[0] << 56 |
         (uint64_t)p[1] << 48 |
         (uint64_t)p[2] << 40 |
	 (uint64_t)p[3] << 32 |
         (uint64_t)p[4] << 24 |
         (uint64_t)p[5] << 16 |
         (uint64_t)p[6] <<  8 |
         (uint64_t)p[7] <<  0;

I suspect compilers would end up with the same output either way (on
x86, "gcc -O2" actually turns the whole thing into a bswap instruction).

-Peff