Re: [XEN PATCH v2] xen/include: avoid undefined behavior.

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 21/06/23 10:48, Jan Beulich wrote:
> On 21.06.2023 09:58, Nicola Vetrini wrote:
> > Redefine BUILD_BUG_ON_ZERO to fully comply with C99 avoiding
> > undefined behavior 58 ("A structure or union is defined as
> > containing no named members (6.7.2.1)."
> Here and in the title I'm not happy about you referencing undefined
> behavior. What we do here is use a well-known compiler extension (and I'm
> sure you're aware we do so elsewhere, where it's actually going to remain
> as is from all I can tell right now).
Noted, I'll change the commit message.

> > --- a/xen/include/xen/lib.h
> > +++ b/xen/include/xen/lib.h
> > @@ -51,9 +51,10 @@
> >      e.g. in a structure initializer (or where-ever else comma expressions
> >      aren't permitted). */
> >   #define BUILD_BUG_ON_ZERO(cond) \
> > -    sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); })
> > +    (sizeof(struct { char c; _Static_assert(!(cond), "!(" #cond ")"); }) - 1U)
> To be compatible with whatever odd ABIs new ports may have, maybe better to
> AND or multiply with 0? (I also don't think a U suffix is warranted here.)
Good idea

> >   #else
> > -#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
> > +#define BUILD_BUG_ON_ZERO(cond) \
> > +    (sizeof(struct { unsigned u : (cond) ? -1 : sizeof(unsigned) * 8; }) - 
> > sizeof(unsigned))
> What's wrong with just giving the bitfield a name:
>
> #define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int _:-!!(cond); })
A named bitfield with zero width is not allowed by the standard, which 
is why the fix introduces a constant positive width. I'll send a v3 
shortly addressing your previous remarks, though.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)