Re: [XEN PATCH][for-4.19 v2 8/8] xen/compat: use BUILD_BUG_ON in CHECK_SIZE macros

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 17/10/2023 08:09, Jan Beulich wrote:
> On 12.10.2023 17:28, Nicola Vetrini wrote:
> > BUILD_BUG_ON is the preferred way to induce a build error
> > upon statically determined incorrect conditions.
> >
> > This also fixes a MISRA C:2012 Rule 10.1 violation in the
> > previous formulation.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> Hmm, looking back it's indeed not clear why I didn't use BUILD_BUG_ON() 
> right
> away. Perhaps just to avoid inline functions when things can be done 
> without.
> And/or because originally the macros were intended to be usable in 
> function
> bodies, not (just) at file scope. However, ...
>
> > --- a/xen/include/xen/compat.h
> > +++ b/xen/include/xen/compat.h
> > @@ -151,12 +151,20 @@ CHECK_NAME_(k, n, T)(k xen_ ## n *x, \
> >      return x == c; \
> >  }
> >
> > -#define CHECK_SIZE(name) \
> > -    typedef int CHECK_NAME(name, S)[1 - (sizeof(xen_ ## name ## _t) 
> > != \
> > -                                         sizeof(compat_ ## name ## 
> > _t)) * 2]
> > +#define CHECK_SIZE(name)                                  \
> > +static inline void __maybe_unused CHECK_SIZE_##name(void) \
> > +{                                                         \
> > +    typedef int CHECK_NAME(name, S)[1];                   \
> ... what's this and ...
>
> > +    BUILD_BUG_ON(sizeof(xen_ ## name ## _t) !=            \
> > +                 sizeof(compat_ ## name ## _t));          \
> > +}
> >  #define CHECK_SIZE_(k, n) \
> > -    typedef int CHECK_NAME_(k, n, S)[1 - (sizeof(k xen_ ## n) != \
> > -                                          sizeof(k compat_ ## n)) * 
> > 2]
> > +static inline void __maybe_unused CHECK_SIZE_##k_##n(void) \
> > +{                                                          \
> > +    typedef int CHECK_NAME_(k, n, S)[1];                   \
> ... this needed for? The types aren't used anywhere afaict.
>
> Jan
>
> > +    BUILD_BUG_ON(sizeof(k xen_ ## n) !=                    \
> > +                 sizeof(k compat_ ## n));                  \
> > +}
> >
> >  #define CHECK_FIELD_COMMON(name, t, f) \
> >  static inline int __maybe_unused name(xen_ ## t ## _t *x, compat_ ## 
> > t ## _t *c) \
You're probably right. I was wondering the same thing when replacing the 
code with
BUILD_BUG_ON.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)