| Message ID | 1507650771-16631-5-git-send-email-volodymyr_babchuk@epam.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 10.10.17 at 17:52, <volodymyr_babchuk@epam.com> wrote: > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -930,6 +930,39 @@ __DEFINE_XEN_GUEST_HANDLE(uint16, uint16_t); > __DEFINE_XEN_GUEST_HANDLE(uint32, uint32_t); > __DEFINE_XEN_GUEST_HANDLE(uint64, uint64_t); > > +typedef struct > +{ Please put the opening brace on the previous line. > + 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__? > +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6) \ > + (xen_uuid_t)XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6) This being (generally) usable as expression, I think it needs parentheses, so e.g. XEN_DEFINE_UUID(...).a[n] becomes possible (whatever that may be good for). Jan
Hello Jan, On 10.10.17 19:12, Jan Beulich wrote: >>>> On 10.10.17 at 17:52, <volodymyr_babchuk@epam.com> wrote: >> --- a/xen/include/public/xen.h >> +++ b/xen/include/public/xen.h >> @@ -930,6 +930,39 @@ __DEFINE_XEN_GUEST_HANDLE(uint16, uint16_t); >> __DEFINE_XEN_GUEST_HANDLE(uint32, uint32_t); >> __DEFINE_XEN_GUEST_HANDLE(uint64, uint64_t); >> >> +typedef struct >> +{ > > Please put the opening brace on the previous line. Will fix. >> + 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__ >= 199901" in various places in XEN, so I did it alike. Also, I'm using C99 feature, not gcc-only one. >> +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6) \ >> + (xen_uuid_t)XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6) > > This being (generally) usable as expression, I think it needs > parentheses, so e.g. XEN_DEFINE_UUID(...).a[n] becomes > possible (whatever that may be good for). Good point, thanks.
>>> 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__)". > >= 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
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 >
>>> On 11.10.17 at 14:12, <volodymyr_babchuk@epam.com> wrote: > 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. Why would that be a bug? In C89 you simply have no way to express what we want, so people using plain C89 compilers will have to code differently (i.e. without using the optarr field) anyway. > 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) That's not very reasonable indeed. > 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. Did you check under what conditions the various gcc versions define __STDC_VERSION__? Trying to compile int test(void) { return __STDC_VERSION__; } with e.g. gcc 4.3.4 fails (with no other options specified). Otoh even using -ansi compound literals compile fine. Jan
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 2ac6b1e..1a6255f 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -930,6 +930,39 @@ __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; + +/* + * 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 +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6) \ + (xen_uuid_t)XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6) +#else +#define XEN_DEFINE_UUID(a, b, c, d, e1, e2, e3, e4, e5, e6) \ + XEN_DEFINE_UUID_(a, b, c, d, e1, e2, e3, e4, e5, e6) +#endif /* defined (__STDC_VERSION__) && __STDC_VERSION__ >= 199901L */ + #endif /* !__ASSEMBLY__ */ /* Default definitions for macros used by domctl/sysctl. */
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, 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: We define a new structure here rather than re-using EFI_GUID. EFI_GUID uses a Microsoft-style encoding which, among other things, mixes little-endian and big-endian. The structure defined in this patch, unlike EFI_GUID, is compatible with the Linux kernel and libuuid. Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com> --- * Changed commit message ("NB:" part). Thanks to George Dunlap for suggestion. --- xen/include/public/xen.h | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+)