Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 27 Feb 2019, Jan Beulich wrote:
> >>> On 26.02.19 at 19:43, <sstabellini@xxxxxxxxxx> wrote:
> > On Tue, 26 Feb 2019, Ian Jackson wrote:
> >> Jan Beulich writes ("Re: [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL"):
> >> > On 26.02.19 at 17:46, <ian.jackson@xxxxxxxxxx> wrote:
> >> > > I am not aware of a standard C type which could be used instead of
> >> > > this struct.  But I think you can use the `packed' attribute to get
> >> > > the right behaviour.  The GCC manual says:
> >> > > 
> >> > >  | Alignment can be decreased by specifying the 'packed' attribute.
> >> > >  | See below.
> >> ...
> >> > Until I've looked at this (again) now, I wasn't even aware that
> >> > one can combine packed and aligned attributes in a sensible
> >> > way. May I suggest that, because of this being a theoretical
> >> > issue only at this point, we limit ourselves to the build time
> >> > assertion you suggest?
> >> 
> >> I am not suggesting combining `packed' and `aligned'.  I am suggesting
> >> only `packed' (but based on text which is in the manual section for
> >> `aligned').  But I am happy with a build-time assertion if you don't
> >> want to add `packed'.  That is just as safe.
> > 
> > Could you please provide a rough example of the build-time assertion you
> > are thinking about? I am happy to add it.
> 
> BUILD_BUG_ON(alignof(*s1) != alignof(*s2));

Thanks! I noticed that BUILD_BUG_ON requires xen/lib.h which cannot be
included from xen/compiler.h. Actually, we were already erroneously
using BUILD_BUG_ON_ZERO in xen/compiler.h without including xen/lib.h.

My suggestion would be to move the definitions of BUG_ON, WARN_ON,
BUILD_BUG_ON and BUILD_BUG_ON_ZERO from xen/lib.h to compiler.h.
Everything works fine if I do that, and it seems a better fit.

Are you OK with this?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel