| Message ID | 20190830083225.10397-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: add macro for defining variable length array in public headers | expand |
On 30.08.2019 10:32, Juergen Gross wrote: > --- a/xen/include/public/xen.h > +++ b/xen/include/public/xen.h > @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); > DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); > DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); > > +/* Define a variable length array (depends on compiler). */ > +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L > +#define __XEN_VARLEN_ARRAY_SIZE > +#elif defined(__GNUC__) > +#define __XEN_VARLEN_ARRAY_SIZE 0 > +#else > +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ > +#endif To be in line with the intentions, the C90 standard, and io/ring.h I'd suggest to use FLEX instead of VARLEN. Furthermore, especially in a public header, two double leading underscores need to go away. And then, with FLEX in the name, SIZE isn't really appropriate anymore. Therefore I see three possible names: XEN_FLEXIBLE_ARRAY, XEN_FLEX_ARRAY_DIMENSION (possibly just _DIM at its end), or XEN_FLEX_ARRAY_DESIGNATOR. Jan
On 30.08.19 16:21, Jan Beulich wrote: > On 30.08.2019 10:32, Juergen Gross wrote: >> --- a/xen/include/public/xen.h >> +++ b/xen/include/public/xen.h >> @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); >> DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); >> DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); >> >> +/* Define a variable length array (depends on compiler). */ >> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L >> +#define __XEN_VARLEN_ARRAY_SIZE >> +#elif defined(__GNUC__) >> +#define __XEN_VARLEN_ARRAY_SIZE 0 >> +#else >> +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ >> +#endif > > To be in line with the intentions, the C90 standard, and io/ring.h > I'd suggest to use FLEX instead of VARLEN. Furthermore, especially > in a public header, two double leading underscores need to go away. > > And then, with FLEX in the name, SIZE isn't really appropriate > anymore. Therefore I see three possible names: XEN_FLEXIBLE_ARRAY, > XEN_FLEX_ARRAY_DIMENSION (possibly just _DIM at its end), or > XEN_FLEX_ARRAY_DESIGNATOR. Okay to the "FLEX" part, but the "XEN_" prefix is not working due to compat header generation (that will result in COMPAT_ prefix in the compat headers). So any other ideas for a sensible prefix? Juergen
On 30.08.2019 16:29, Juergen Gross wrote: > On 30.08.19 16:21, Jan Beulich wrote: >> On 30.08.2019 10:32, Juergen Gross wrote: >>> --- a/xen/include/public/xen.h >>> +++ b/xen/include/public/xen.h >>> @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); >>> DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); >>> DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); >>> >>> +/* Define a variable length array (depends on compiler). */ >>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L >>> +#define __XEN_VARLEN_ARRAY_SIZE >>> +#elif defined(__GNUC__) >>> +#define __XEN_VARLEN_ARRAY_SIZE 0 >>> +#else >>> +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ >>> +#endif >> >> To be in line with the intentions, the C90 standard, and io/ring.h >> I'd suggest to use FLEX instead of VARLEN. Furthermore, especially >> in a public header, two double leading underscores need to go away. >> >> And then, with FLEX in the name, SIZE isn't really appropriate >> anymore. Therefore I see three possible names: XEN_FLEXIBLE_ARRAY, >> XEN_FLEX_ARRAY_DIMENSION (possibly just _DIM at its end), or >> XEN_FLEX_ARRAY_DESIGNATOR. > > Okay to the "FLEX" part, but the "XEN_" prefix is not working due to > compat header generation (that will result in COMPAT_ prefix in the > compat headers). > > So any other ideas for a sensible prefix? Hmm, ugly. Perhaps all lower case then? Or have #define COMPAT_FLEX_... XEN_FLEX_... somewhere? Jan
On 30.08.19 17:22, Jan Beulich wrote: > On 30.08.2019 16:29, Juergen Gross wrote: >> On 30.08.19 16:21, Jan Beulich wrote: >>> On 30.08.2019 10:32, Juergen Gross wrote: >>>> --- a/xen/include/public/xen.h >>>> +++ b/xen/include/public/xen.h >>>> @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); >>>> DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); >>>> DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); >>>> >>>> +/* Define a variable length array (depends on compiler). */ >>>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L >>>> +#define __XEN_VARLEN_ARRAY_SIZE >>>> +#elif defined(__GNUC__) >>>> +#define __XEN_VARLEN_ARRAY_SIZE 0 >>>> +#else >>>> +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ >>>> +#endif >>> >>> To be in line with the intentions, the C90 standard, and io/ring.h >>> I'd suggest to use FLEX instead of VARLEN. Furthermore, especially >>> in a public header, two double leading underscores need to go away. >>> >>> And then, with FLEX in the name, SIZE isn't really appropriate >>> anymore. Therefore I see three possible names: XEN_FLEXIBLE_ARRAY, >>> XEN_FLEX_ARRAY_DIMENSION (possibly just _DIM at its end), or >>> XEN_FLEX_ARRAY_DESIGNATOR. >> >> Okay to the "FLEX" part, but the "XEN_" prefix is not working due to >> compat header generation (that will result in COMPAT_ prefix in the >> compat headers). >> >> So any other ideas for a sensible prefix? > > Hmm, ugly. Perhaps all lower case then? Or have xen_ and Xen_ are being converted, too. > > #define COMPAT_FLEX_... XEN_FLEX_... > > somewhere? Seems as if doing that in include/public/xen-compat.h is working. Juergen
On 30.08.2019 17:30, Juergen Gross wrote: > On 30.08.19 17:22, Jan Beulich wrote: >> On 30.08.2019 16:29, Juergen Gross wrote: >>> On 30.08.19 16:21, Jan Beulich wrote: >>>> On 30.08.2019 10:32, Juergen Gross wrote: >>>>> --- a/xen/include/public/xen.h >>>>> +++ b/xen/include/public/xen.h >>>>> @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); >>>>> DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); >>>>> DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); >>>>> >>>>> +/* Define a variable length array (depends on compiler). */ >>>>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L >>>>> +#define __XEN_VARLEN_ARRAY_SIZE >>>>> +#elif defined(__GNUC__) >>>>> +#define __XEN_VARLEN_ARRAY_SIZE 0 >>>>> +#else >>>>> +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ >>>>> +#endif >>>> >>>> To be in line with the intentions, the C90 standard, and io/ring.h >>>> I'd suggest to use FLEX instead of VARLEN. Furthermore, especially >>>> in a public header, two double leading underscores need to go away. >>>> >>>> And then, with FLEX in the name, SIZE isn't really appropriate >>>> anymore. Therefore I see three possible names: XEN_FLEXIBLE_ARRAY, >>>> XEN_FLEX_ARRAY_DIMENSION (possibly just _DIM at its end), or >>>> XEN_FLEX_ARRAY_DESIGNATOR. >>> >>> Okay to the "FLEX" part, but the "XEN_" prefix is not working due to >>> compat header generation (that will result in COMPAT_ prefix in the >>> compat headers). >>> >>> So any other ideas for a sensible prefix? >> >> Hmm, ugly. Perhaps all lower case then? Or have > > xen_ and Xen_ are being converted, too. For Xen_ I agree, but for xen_ I only see [ r"(struct|union|enum)\s+(xen_?)?(\w)", r"\1 compat_\3" ], and [ r"(^|[^\w])xen_?(\w*)_compat_t([^\w]|$$)", r"\1compat_\2_t\3" ], , neither of which should match your #define. But perhaps I'm missing something. Jan
diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h index 8344aa471f..7cb6952ab8 100644 --- a/xen/include/public/arch-x86/hvm/save.h +++ b/xen/include/public/arch-x86/hvm/save.h @@ -632,13 +632,7 @@ struct hvm_msr { uint32_t index; uint32_t _rsvd; uint64_t val; -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - } msr[]; -#elif defined(__GNUC__) - } msr[0]; -#else - } msr[1 /* variable size */]; -#endif + } msr[__XEN_VARLEN_ARRAY_SIZE]; }; #define CPU_MSR_CODE 20 diff --git a/xen/include/public/arch-x86/pmu.h b/xen/include/public/arch-x86/pmu.h index 68ebf121d0..6c3b8cd4b6 100644 --- a/xen/include/public/arch-x86/pmu.h +++ b/xen/include/public/arch-x86/pmu.h @@ -35,11 +35,7 @@ struct xen_pmu_amd_ctxt { uint32_t ctrls; /* Counter MSRs */ -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - uint64_t regs[]; -#elif defined(__GNUC__) - uint64_t regs[0]; -#endif + uint64_t regs[__XEN_VARLEN_ARRAY_SIZE]; }; typedef struct xen_pmu_amd_ctxt xen_pmu_amd_ctxt_t; DEFINE_XEN_GUEST_HANDLE(xen_pmu_amd_ctxt_t); @@ -71,11 +67,7 @@ struct xen_pmu_intel_ctxt { uint64_t debugctl; /* Fixed and architectural counter MSRs */ -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - uint64_t regs[]; -#elif defined(__GNUC__) - uint64_t regs[0]; -#endif + uint64_t regs[__XEN_VARLEN_ARRAY_SIZE]; }; typedef struct xen_pmu_intel_ctxt xen_pmu_intel_ctxt_t; DEFINE_XEN_GUEST_HANDLE(xen_pmu_intel_ctxt_t); diff --git a/xen/include/public/argo.h b/xen/include/public/argo.h index cc603d395d..696b227137 100644 --- a/xen/include/public/argo.h +++ b/xen/include/public/argo.h @@ -82,11 +82,7 @@ typedef struct xen_argo_ring * multiple of the message slot size. */ uint8_t reserved[56]; -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - uint8_t ring[]; -#elif defined(__GNUC__) - uint8_t ring[0]; -#endif + uint8_t ring[__XEN_VARLEN_ARRAY_SIZE]; } xen_argo_ring_t; typedef struct xen_argo_register_ring @@ -136,11 +132,7 @@ typedef struct xen_argo_ring_data { uint32_t nent; uint32_t pad; -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - struct xen_argo_ring_data_ent data[]; -#elif defined(__GNUC__) - struct xen_argo_ring_data_ent data[0]; -#endif + struct xen_argo_ring_data_ent data[__XEN_VARLEN_ARRAY_SIZE]; } xen_argo_ring_data_t; struct xen_argo_ring_message_header @@ -148,11 +140,7 @@ struct xen_argo_ring_message_header uint32_t len; struct xen_argo_addr source; uint32_t message_type; -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - uint8_t data[]; -#elif defined(__GNUC__) - uint8_t data[0]; -#endif + uint8_t data[__XEN_VARLEN_ARRAY_SIZE]; }; /* diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h index b6faf8350c..c20c4451d3 100644 --- a/xen/include/public/physdev.h +++ b/xen/include/public/physdev.h @@ -300,11 +300,7 @@ struct physdev_pci_device_add { * First element ([0]) is PXM domain associated with the device (if * XEN_PCI_DEV_PXM is set) */ -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - uint32_t optarr[]; -#elif defined(__GNUC__) - uint32_t optarr[0]; -#endif + uint32_t optarr[__XEN_VARLEN_ARRAY_SIZE]; }; typedef struct physdev_pci_device_add physdev_pci_device_add_t; DEFINE_XEN_GUEST_HANDLE(physdev_pci_device_add_t); diff --git a/xen/include/public/version.h b/xen/include/public/version.h index 7063e8ca55..4cdab53872 100644 --- a/xen/include/public/version.h +++ b/xen/include/public/version.h @@ -95,11 +95,8 @@ typedef char xen_commandline_t[1024]; #define XENVER_build_id 10 struct xen_build_id { uint32_t len; /* IN: size of buf[]. */ -#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L - unsigned char buf[]; -#elif defined(__GNUC__) - unsigned char buf[1]; /* OUT: Variable length buffer with build_id. */ -#endif + unsigned char buf[__XEN_VARLEN_ARRAY_SIZE]; + /* OUT: Variable length buffer with build_id. */ }; typedef struct xen_build_id xen_build_id_t; diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 0c7b5c3865..7b31f68043 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -53,6 +53,15 @@ DEFINE_XEN_GUEST_HANDLE(uint64_t); DEFINE_XEN_GUEST_HANDLE(xen_pfn_t); DEFINE_XEN_GUEST_HANDLE(xen_ulong_t); +/* Define a variable length array (depends on compiler). */ +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L +#define __XEN_VARLEN_ARRAY_SIZE +#elif defined(__GNUC__) +#define __XEN_VARLEN_ARRAY_SIZE 0 +#else +#define __XEN_VARLEN_ARRAY_SIZE 1 /* variable size */ +#endif + /* Turn a plain number into a C unsigned (long (long)) constant. */ #define __xen_mk_uint(x) x ## U #define __xen_mk_ulong(x) x ## UL
Several public headers of the hypervisor contain structures with variable length arrays. In order to be usable with different compilers those definitions are depending on the compiler type and the standard supported by the compiler. In order to avoid open coding the different variants in each header add a common macro for that purpose in xen.h. This at once corrects most of the definitions which miss one case leading to not defining the array at all. Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/include/public/arch-x86/hvm/save.h | 8 +------- xen/include/public/arch-x86/pmu.h | 12 ++---------- xen/include/public/argo.h | 18 +++--------------- xen/include/public/physdev.h | 6 +----- xen/include/public/version.h | 7 ++----- xen/include/public/xen.h | 9 +++++++++ 6 files changed, 18 insertions(+), 42 deletions(-)