Re: [XEN PATCH 2/4] x86/hvm: address violations of MISRA C Rule 20.7

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

On 2024-05-16 01:18, Stefano Stabellini wrote:
> On Wed, 15 May 2024, 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>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
>> ---
>>  xen/arch/x86/hvm/mtrr.c             | 2 +-
>>  xen/arch/x86/hvm/rtc.c              | 2 +-
>>  xen/arch/x86/include/asm/hvm/save.h | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
>> index 32f74c1db03b..1079851f70ed 100644
>> --- a/xen/arch/x86/hvm/mtrr.c
>> +++ b/xen/arch/x86/hvm/mtrr.c
>> @@ -16,7 +16,7 @@
>>  #include <public/hvm/e820.h>
>> 
>>  /* Get page attribute fields (PAn) from PAT MSR. */
>> -#define pat_cr_2_paf(pat_cr,n)  ((((uint64_t)pat_cr) >> ((n)<<3)) & 
>> 0xff)
>> +#define pat_cr_2_paf(pat_cr, n)  ((((uint64_t)(pat_cr)) >> ((n) << 
>> 3)) & 0xff)
> 
> just a cosmetic change
> 

No. The point here is that pat_cr could in principle be something like 1 
+ 2, which without parentheses may not expand as intended (in this case 
because of the nearby cast). Then there's also the style fix on n

> 
>>  /* Effective mm type lookup table, according to MTRR and PAT. */
>>  static const uint8_t mm_type_tbl[MTRR_NUM_TYPES][X86_NUM_MT] = {
>> diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c
>> index 4bb1c7505534..72c7bdbfcd02 100644
>> --- a/xen/arch/x86/hvm/rtc.c
>> +++ b/xen/arch/x86/hvm/rtc.c
>> @@ -45,7 +45,7 @@
>>  #define vrtc_domain(x) (container_of(x, struct pl_time, 
>> vrtc)->domain)
>>  #define vrtc_vcpu(x)   (pt_global_vcpu_target(vrtc_domain(x)))
>>  #define epoch_year     1900
>> -#define get_year(x)    (x + epoch_year)
>> +#define get_year(x)    ((x) + epoch_year)
>> 
>>  enum rtc_mode {
>>     rtc_mode_no_ack,
>> diff --git a/xen/arch/x86/include/asm/hvm/save.h 
>> b/xen/arch/x86/include/asm/hvm/save.h
>> index 8149aa113cb4..ec8de029319d 100644
>> --- a/xen/arch/x86/include/asm/hvm/save.h
>> +++ b/xen/arch/x86/include/asm/hvm/save.h
>> @@ -50,7 +50,7 @@ int _hvm_check_entry(struct hvm_domain_context *h,
>>                            HVM_SAVE_LENGTH(x), true) == 0 )      \
>>      {                                                           \
>>          ptr = &(h)->data[(h)->cur];                             \
>> -        h->cur += HVM_SAVE_LENGTH(x);                           \
>> +        (h)->cur += HVM_SAVE_LENGTH(x);                         \
>>      }                                                           \
>>      ptr; })
>> 
>> --
>> 2.34.1
>> 

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