[Spice-devel] [PATCH v2 2/2] Define and use new SPICE_MAGIC_CONST macro

[Frediano Ziglio]

> On Tue, Aug 11, 2015 at 11:14:07AM -0400, 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 at redhat.com>
> > ---
> > 
> > Did a mistake with the macros (parenthesis mismatch).
> > Tested the method also with Visual Studio 2008 compiler and works as
> > expected.
> > 
> > 
> >  spice/controller_prot.h   | 2 +-
> >  spice/foreign_menu_prot.h | 2 +-
> >  spice/macros.h            | 8 ++++++++
> >  spice/protocol.h          | 2 +-
> >  spice/qxl_dev.h           | 4 ++--
> >  spice/stats.h             | 2 +-
> >  spice/vdi_dev.h           | 2 +-
> >  7 files changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/spice/controller_prot.h b/spice/controller_prot.h
> > index bca7804..a21bf57 100644
> > --- a/spice/controller_prot.h
> > +++ b/spice/controller_prot.h
> > @@ -21,7 +21,7 @@
> >  #include <spice/types.h>
> >  #include <spice/start-packed.h>
> >  
> > -#define CONTROLLER_MAGIC      (*(uint32_t*)"CTRL")
> > +#define CONTROLLER_MAGIC      SPICE_MAGIC_CONST("CTRL")
> >  #define CONTROLLER_VERSION    1
> >  
> >  
> > diff --git a/spice/foreign_menu_prot.h b/spice/foreign_menu_prot.h
> > index f478e2a..9dd882d 100644
> > --- a/spice/foreign_menu_prot.h
> > +++ b/spice/foreign_menu_prot.h
> > @@ -21,7 +21,7 @@
> >  #include <spice/types.h>
> >  #include <spice/start-packed.h>
> >  
> > -#define FOREIGN_MENU_MAGIC      (*(uint32_t*)"FRGM")
> > +#define FOREIGN_MENU_MAGIC      SPICE_MAGIC_CONST("FRGM")
> >  #define FOREIGN_MENU_VERSION    1
> >  
> >  typedef struct SPICE_ATTR_PACKED FrgMenuInitHeader {
> > diff --git a/spice/macros.h b/spice/macros.h
> > index 3538989..679d68d 100644
> > --- a/spice/macros.h
> > +++ b/spice/macros.h
> > @@ -414,4 +414,12 @@
> >  #endif
> >  
> >  
> > +#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
> 
> Is it expected that 3rd member (the one shifted by 16 bits) does not
> have 0xffu ? Apart from this, ACK, so I can fix and push.
> 
> Christophe
> 

Good catch. Yes, for coherence 0xffu would be better.

actually CHAR&0xff|UNSIGNED is ((unsigned)((int)char)&0xff))|UNSIGNED
while CHAR&0xffu|UNSIGNED is (((unsigned)char)&0xffu)|UNSIGNED
so looking at processor implementations should be same assembly code but
better to be source consistent.

Frediano