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

Reply via email to