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

[Jan Beulich <jbeulich@xxxxxxxx>]

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 ...

> --- 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