Re: [Xen-devel] [PATCH v6 03/10] public: xen.h: add definitions for UUID handling

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 21/09/17 20:59, Volodymyr Babchuk wrote:
> Added type xen_uuid_t. This type represents UUID as an array of 16
> bytes in big endian format.
>
> Added macro XEN_DEFINE_UUID that constructs UUID in the usual way:
>
>   XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899, 0xaabbccddeeff)
>
> will construct UUID 00112233-4455-6677-8899-aabbccddeeff presented as
>   {0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88,
>    0x99, 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff}
>
> NB: This is compatible with Linux kernel and with libuuid, but it is not
> compatible with Microsoft, as they use mixed-endian encoding (some
> components are little-endian, some are big-endian).
I think this paragraph should also be duplicated in the header to help 
the user.
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> ---
>
>   * Fixed commit message (added 0x in XEN_DEFINE_UUID usage example)
>   * As was agreed with Julien Grail ([1]), added two flavours of
>     XEN_DEFINE_UUID(). One of them casts initialization list
>     to right type (if compiler supports this), another - does not.
>
> [1] http://markmail.org/message/cpced37242alfvz7
> ---
>   xen/include/public/xen.h | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index 2ac6b1e..263bb3b 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -930,6 +930,33 @@ __DEFINE_XEN_GUEST_HANDLE(uint16, uint16_t);
>   __DEFINE_XEN_GUEST_HANDLE(uint32, uint32_t);
>   __DEFINE_XEN_GUEST_HANDLE(uint64, uint64_t);
>   
> +typedef struct
> +{
> +    uint8_t a[16];
> +} xen_uuid_t;
> +
> +#if defined(__GNUC__) && !defined(__STRICT_ANSI__)
> +
> +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)             \
> +    (xen_uuid_t){{((a) >> 24) & 0xFF, ((a) >> 16) & 0xFF,               \
> +                  ((a) >>  8) & 0xFF, ((a) >>  0) & 0xFF,               \
> +                  ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,               \
> +                  ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,               \
> +                  ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,               \
> +                           e1, e2, e3, e4, e5, e6}}
> +
> +#else
> +
> +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)             \
> +    {{((a) >> 24) & 0xFF, ((a) >> 16) & 0xFF,                           \
> +      ((a) >>  8) & 0xFF, ((a) >>  0) & 0xFF,                           \
> +      ((b) >>  8) & 0xFF, ((b) >>  0) & 0xFF,                           \
> +      ((c) >>  8) & 0xFF, ((c) >>  0) & 0xFF,                           \
> +      ((d) >>  8) & 0xFF, ((d) >>  0) & 0xFF,                           \
> +                e1, e2, e3, e4, e5, e6}}
The only difference between the two macros is the former is having the 
cast whilst the latter not.
How about factorizing the code, i.e:

#define __XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6)
     {{ ... }}

#if defined(....)

#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6) \
   (xen_uuid_t)__XEN_DEFINE_UUID(a, b, c, d, ...)

#else

#define XEN_DEFINE_UUID(....) __XEN_DEFINE_UUID(...)

Any opinons?

> +
> +#endif /* (__GNUC__) && !defined(__STRICT_ANSI__) */
> +
>   #endif /* !__ASSEMBLY__ */
>   
>   /* Default definitions for macros used by domctl/sysctl. */
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel