On Sat, Sep 23, 2017 at 11:31:50PM +0000, Ben Peart wrote:
> > 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
