Re: [PATCH v8 04/11] public: xen.h: add definitions for UUID handling

[Volodymyr Babchuk <volodymyr_babchuk@epam.com>]

Hi jan

On 11.10.17 11:07, Jan Beulich wrote:
>>>> On 10.10.17 at 19:03, <volodymyr_babchuk@epam.com> wrote:
>> On 10.10.17 19:12, Jan Beulich wrote:
>>>>>> On 10.10.17 at 17:52, <volodymyr_babchuk@epam.com> wrote:
>>>> +    uint8_t a[16];
>>>> +} xen_uuid_t;
>>>> +
>>>> +/*
>>>> + * XEN_DEFINE_UUID(0x00112233, 0x4455, 0x6677, 0x8899,
>>>> + *                 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff)
>>>> + * 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).
>>>> + */
>>>> +#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}}
>>>> +
>>>> +/* Compound literals are supported in C99 and later. */
>>>> +#if defined (__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
>>>
>>> I didn't notice this earlier - why no check for __GNUC__?
>> I have seen pattern "#if defined (__STDC_VERSION__) && __STDC_VERSION__
> 
> But if you check, all of the existing ones have
> "#elif defined(__GNUC__)".
Yes. But there was a reason do to so. For example:

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
     uint32_t optarr[];
#elif defined(__GNUC__)
     uint32_t optarr[0];
#endif

(xen/include/public/physdev.h:303)

If compiler is C99 then we use flexible length array, else if compiller 
is GCC, we use zero-length array, which is GCC extension  (correct me). 
Other compilers (non-gcc C90 compatible) are not supported. Probably 
this is a bug.

Another case is even worse:

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
         unsigned char   buf[];
#elif defined(__GNUC__)
         unsigned char   buf[1]; /* OUT: Variable length buffer with 
build_id. */

(xen/include/public/version.h:49)

Array of length one is perfectly fine in any dialect of C. There are no 
need to check for GCC.
Also, again, this code will not define `buf` on non-gcc, C90 compatible 
compilers.

My code does not use gcc-only extensions like zero-length arrays, so I 
don't see how #elif defined (__GNUC__) can fit in the my case.


>>   >= 199901" in various places in XEN, so I did it alike.
>> Also, I'm using C99 feature, not gcc-only one.
> 
> I didn't ask you to replace the conditional, but to (possibly)
> extend it.
> 
> Jan
> 

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