Re: Empty struct in public headers Was: Re: [PATCH for Xen 4.5] xen/arm: Add support for GICv3 for domU

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2014-11-18 at 16:29 +0000, Julien Grall wrote:
> On 11/18/2014 04:21 PM, Ian Campbell wrote:
> > On Tue, 2014-11-18 at 15:49 +0000, Julien Grall wrote:
> >> (Rename the mail and strip the cc list)
> >>
> >> On 11/18/2014 03:35 PM, Ian Jackson wrote:
> >>> Julien Grall writes ("Re: [PATCH for Xen 4.5] xen/arm: Add support for GICv3 for domU"):
> >>>> On 11/18/2014 03:10 PM, Ian Jackson wrote:
> >>>>> Empty structs are a gcc extension (`(gcc-4.4) Empty Structures').  I
> >>>>> would be very surprised if clang didn't support them too.
> >>>>
> >>>> AFAIK, clang doesn't complain about empty structures.
> >>>
> >>> Right.
> >>>
> >>>>> AIUI our policy, gcc extensions are fine except in the Xen public
> >>>>> headers.
> >>>>
> >>>> We have at least 2 "empty" structure on the ARM public header.
> >>>
> >>> That ought to be fixed, in case anyone ever wants to build ARM guests
> >>> with Norcroft C or something.
> >>>
> >>> Does the size of these structs matter ?
> >>
> >> The 2 structures are arch_vcpu_info and arch_shared_info.
> >>
> >> They are used only at the end of the structure vcpu_info (resp.
> >> shared_info). So I guess we could fix it?
> > 
> > arch_vcpu_info isn't at the end of vcpu_info (vcpu_time_info follows it)
> > and also vcpu_info is part of an array at the start of shared_info (an
> > array of 1 on ARM, but things still follow it). I'm also not sure of the
> > impact on the vcpu placement hypercall or the uses of it.
> 
> Oops, right. I looked too quickly to the code, sorry.
> 
> > So it looks like changing vcpu_info at least will be hard/impossible. If
> > we want rid of these empty structs then I think an ifdef at the point of
> > use is the only option :-(
> 
> I will send a patch to ifdef arch_vcpu and arch_shared_info when Xen 4.6
> windows will be opened.

Sounds good. The approach we've used in this area before is XEN_HAVE_FOO
(e.g. PV_UPCALL_MASK), so I suppose XEN_HAVE_ARCH_VCPU and
XEN_HAVE_ARCH_SHARED_INFO defined for x86 would be the most consistent
way to go.

Ian.