Re: [XEN PATCH v2 2/3] xen/unaligned: address violation of MISRA C Rule 20.7

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-04-30 17:13, Jan Beulich wrote:
> On 30.04.2024 16:28, Nicola Vetrini wrote:
> > MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> > of macro parameters shall be enclosed in parentheses". Therefore, some
> > macro definitions should gain additional parentheses to ensure that 
> > all
> > current and future users will be safe with respect to expansions that
> > can possibly alter the semantics of the passed-in macro parameter.
> >
> > No functional change.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> > Somewhat surprisingly, the change in the tools directory is also 
> > needed, otherwise
> > some CI build jobs fail (see e.g. [1]). This is not undefined 
> > behaviour
> > as long as the two definitions are kept in sync, following section
> > 6.10.3p2 of the C99 standard, but having the definition in 
> > common-macros.h
> > is still a potential problem.
> >
> > [1] 
> > https://gitlab.com/xen-project/people/bugseng/xen/-/jobs/6742878558
> This is pretty absurd, and the use of the Xen header in
> xg_dom_decompress_unsafe_zstd.c should probably have gone away with the
> introduction of the unaligned macros into ...
I agree. I assumed it was known/deliberate.

> > --- a/tools/include/xen-tools/common-macros.h
> > +++ b/tools/include/xen-tools/common-macros.h
> > @@ -102,7 +102,7 @@
> >
> >  #define put_unaligned_t(type, val, ptr) do {                        \
> >      struct { type x; } __packed *ptr_ = (typeof(ptr_))(ptr);        \
> > -    ptr_->x = val;                                                  \
> > +    ptr_->x = (val);                                                \
> >  } while (0)
> >
> >  #define get_unaligned(ptr)      get_unaligned_t(typeof(*(ptr)), ptr)
> ... here. We simply cannot assume the two definitions can indefinitely
> be kept in sync.
>
> > --- a/xen/include/xen/unaligned.h
> > +++ b/xen/include/xen/unaligned.h
> > @@ -19,7 +19,7 @@
> >
> >  #define put_unaligned_t(type, val, ptr) do {                           \
> >         struct { type x; } __packed *ptr_ = (typeof(ptr_))(ptr);        \
> > -       ptr_->x = val;                                                       \
> > +       ptr_->x = (val);                                                     \
> Nit: One of the padding tabs then wants dropping.
>
> Jan
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)