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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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@bugseng.com>
>> ---
>> 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)