Re: [Xen-devel] [PATCH v12 for-xen-4.5 01/20] common/symbols: Export hypervisor symbols to privileged guest

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 09/29/2014 02:43 AM, Jan Beulich wrote:
> > > > On 26.09.14 at 18:49, <konrad.wilk@xxxxxxxxxx> wrote:
> > On Fri, Sep 26, 2014 at 04:10:09PM +0100, Jan Beulich wrote:
> > > > > > On 26.09.14 at 16:58, <konrad.wilk@xxxxxxxxxx> wrote:
> > > > If I move them just a bit:
> > > >
> > > >
> > > > diff --git a/xen/include/public/platform.h b/xen/include/public/platform.h
> > > > index 4f21b17..b97e476 100644
> > > > --- a/xen/include/public/platform.h
> > > > +++ b/xen/include/public/platform.h
> > > > @@ -538,9 +538,9 @@ struct xenpf_symdata {
> > > >                         /*      we reached the end                        */
> > > >   
> > > >       /* OUT variables */
> > > > -    char type;
> > > > -    XEN_GUEST_HANDLE(char) name;
> > > >       uint64_t address;
> > > > +    XEN_GUEST_HANDLE(char) name;
> > > > +    char type;
> > > >   };
> > > >   typedef struct xenpf_symdata xenpf_symdata_t;
> > > >   DEFINE_XEN_GUEST_HANDLE(xenpf_symdata_t);
> > > >
> > > >
> > > > 'pahole' is satisfied:
> > > >
> > > > struct xenpf_symdata {
> >   
> > > >      uint32_t                   namelen;              /*     0     4 */
> >   
> > > >      uint32_t                   symnum;               /*     4     4 */
> >   
> > > >      uint64_t                   address;              /*     8     8 */
> >   
> > > >      __guest_handle_char        name;                 /*    16     8 */
> >   
> > > >      char                       type;                 /*    24     1 */
> >   
> > > >                                                                              
> >   
> > > >      /* size: 32, cachelines: 1, members: 5 */
> >   
> > > >      /* padding: 7 */
> >   
> > > >      /* last cacheline: 32 bytes */
> >   
> > > > };
> > > >
> > > >
> > > > With that change, Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > This change buys us exactly nothing: Structure size doesn't change,
> > > and 7 bytes of padding are still there.
> > It does allow us to put more parameters (if we want to) at the end of the
> > structure instead of fitting them in between.
> Regardless of where the gap is, adding further fields in the future
> would work only if the code now checked that this field is zero
> (which first of all would require it being given a name). I keep
> pointing out that this should be done for all padding fields, but I'm
> afraid I may have missed doing so on this occasion.
I am not sure I understand how setting fields to zero would help with 
figuring out whether a new fields has been added. I can see how it can 
in some cases but not in general.
-boris

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