> > On Fri, Aug 07, 2015 at 01:29:57PM +0100, Frediano Ziglio wrote: > > This macro allow to define magic constants without using weird > > memory tweacks. > > This remove some possible warning from some compiler and > > make code more optimized as compiler is able to compute the > > constant. > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > spice/controller_prot.h | 2 +- > > spice/foreign_menu_prot.h | 2 +- > > spice/macros.h | 6 ++++++ > > spice/protocol.h | 2 +- > > spice/qxl_dev.h | 4 ++-- > > spice/stats.h | 2 +- > > spice/vdi_dev.h | 2 +- > > 7 files changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/spice/macros.h b/spice/macros.h > > index 018ff32..4647ee3 100644 > > --- a/spice/macros.h > > +++ b/spice/macros.h > > @@ -359,5 +359,11 @@ > > # define SPICE_BYTESWAP64(val) (SPICE_BYTESWAP64_CONSTANT (val)) > > #endif /* generic */ > > > > +#if SPICE_ENDIAN == SPICE_ENDIAN_LITTLE > > +#define SPICE_MAGIC_CONST(s) > > ((uint32_t)(s[0]+s[1]*0x100u+s[2]*0x10000u+s[3]*0x1000000u)) > > +#else > > +#define SPICE_MAGIC_CONST(s) > > ((uint32_t)(s[3]+s[2]*0x100u+s[1]*0x10000u+s[0]*0x1000000u)) > > +#endif > > + > > If I'm not mistaken, we can keep the (*(const uint32_t*)"QXRO") method > for little-endian (or are you avoiding -Wcast-align warnings at > the same time)? This could potentially use SPICE_BYTESWAP32_CONSTANT > (but I assume there is still this -Wcast-align warning to take care of?) > What about SPICE_ENDIAN_PDP, I believe this is not going to be correct? > (I would just error out at compile time if we find out the endianness is > PDP). I'd favour << and | over +/* for what it's worth, and make sure we > are operating on unsigned chars. > > Christophe > The (*(const uint32_t*)"QXRO") does not produce a constant (at least on gcc) but a string and a pointer dereference. I don't cast to unsigned char as actually all strings are ASCII (so cast is not necessary). The * instead of << make the code smaller as doing a ((char) c) << 24 just can lead to 0 in all cases while ((char) c) * 0x1000000u force the cast char -> unsigned. There is already the check for SPICE_ENDIAN_PDP. Yes, I can use SPICE_BYTESWAP32_CONSTANT (it just expands in a really long code). A code like this can work #if SPICE_ENDIAN == SPICE_ENDIAN_LITTLE #define SPICE_MAGIC_CONST(s) \ ((uint32_t)((s[0]&0xffu)|((s[1]&0xffu)<<8)|((s[2]&0xff)<<16)|((s[3]&0xffu)<<24)) #else #define SPICE_MAGIC_CONST(s) \ ((uint32_t)((s[3]&0xffu)|((s[2]&0xffu)<<8)|((s[1]&0xff)<<16)|((s[0]&0xffu)<<24)) #endif or #if SPICE_ENDIAN == SPICE_ENDIAN_LITTLE #define SPICE_MAGIC_CONST(s) \ ((uint32_t)((s[0]&0xffu)|((s[1]&0xffu)<<8)|((s[2]&0xff)<<16)|((s[3]&0xffu)<<24)) #else #define SPICE_MAGIC_CONST(s) \ SPICE_BYTESWAP32_CONSTANT((s[0]&0xffu)|((s[1]&0xffu)<<8)|((s[2]&0xff)<<16)|((s[3]&0xffu)<<24)) #endif Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel