[XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[Nicola Vetrini]

Hi all,

this series aims to refactor some macros that cause violations of MISRA C Rule
20.7 ("Expressions resulting from the expansion of macro parameters shall be
enclosed in parentheses"). All the macros touched by these patches are in some
way involved in violations, and the strategy adopted to bring them into
compliance is to add parentheses around macro arguments where needed.

Nicola Vetrini (4):
  x86/vpmu: address violations of MISRA C Rule 20.7
  x86/hvm: address violations of MISRA C Rule 20.7
  x86_64/uaccess: address violations of MISRA C Rule 20.7
  x86_64/cpu_idle: address violations of MISRA C Rule 20.7

 xen/arch/x86/cpu/vpmu_amd.c               | 4 ++--
 xen/arch/x86/hvm/mtrr.c                   | 2 +-
 xen/arch/x86/hvm/rtc.c                    | 2 +-
 xen/arch/x86/include/asm/hvm/save.h       | 2 +-
 xen/arch/x86/include/asm/x86_64/uaccess.h | 7 ++++---
 xen/arch/x86/x86_64/cpu_idle.c            | 2 +-
 6 files changed, 10 insertions(+), 9 deletions(-)

-- 
2.34.1

[XEN PATCH 1/4] x86/vpmu: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

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>
---
 xen/arch/x86/cpu/vpmu_amd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index db2fa420e14a..97e6315bd9f7 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -25,8 +25,8 @@
 
 #define is_guest_mode(msr) ((msr) & (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
 #define is_pmu_enabled(msr) ((msr) & (1ULL << MSR_F10H_EVNTSEL_EN_SHIFT))
-#define set_guest_mode(msr) (msr |= (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
-#define is_overflowed(msr) (!((msr) & (1ULL << (MSR_F10H_COUNTER_LENGTH-1))))
+#define set_guest_mode(msr) ((msr) |= (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
+#define is_overflowed(msr) (!((msr) & (1ULL << (MSR_F10H_COUNTER_LENGTH - 1))))
 
 static unsigned int __read_mostly num_counters;
 static const u32 __read_mostly *counters;
-- 
2.34.1

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

[Stefano Stabellini]

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/cpu/vpmu_amd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
> index db2fa420e14a..97e6315bd9f7 100644
> --- a/xen/arch/x86/cpu/vpmu_amd.c
> +++ b/xen/arch/x86/cpu/vpmu_amd.c
> @@ -25,8 +25,8 @@
>  
>  #define is_guest_mode(msr) ((msr) & (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
>  #define is_pmu_enabled(msr) ((msr) & (1ULL << MSR_F10H_EVNTSEL_EN_SHIFT))
> -#define set_guest_mode(msr) (msr |= (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
> -#define is_overflowed(msr) (!((msr) & (1ULL << (MSR_F10H_COUNTER_LENGTH-1))))
> +#define set_guest_mode(msr) ((msr) |= (1ULL << MSR_F10H_EVNTSEL_GO_SHIFT))
> +#define is_overflowed(msr) (!((msr) & (1ULL << (MSR_F10H_COUNTER_LENGTH - 1))))

is_overflowed is just a cosmetic change

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

[Jan Beulich]

On 16.05.2024 01:17, 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>

Acked-by: Jan Beulich <jbeulich@suse.com>

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

[Nicola Vetrini]

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>
---
 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)
 
 /* 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

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

[Stefano Stabellini]

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


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

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

[Nicola Vetrini]

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)

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

[Jan Beulich]

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

Not really, as Nicola already pointed out. However, there's an excess pair of
parentheses which better would have been re-arranged rather than adding yet
another pair:

#define pat_cr_2_paf(pat_cr, n)  (((uint64_t)(pat_cr) >> ((n) << 3)) & 0xff)

Preferably with that (which I'll likely take the liberty of adjusting while
committing)
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

[XEN PATCH 3/4] x86_64/uaccess: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

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.

xlat_malloc_init is touched for consistency, despite the construct
being already deviated.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/include/asm/x86_64/uaccess.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/x86_64/uaccess.h b/xen/arch/x86/include/asm/x86_64/uaccess.h
index ba79f950fba9..c6fa3fd381bc 100644
--- a/xen/arch/x86/include/asm/x86_64/uaccess.h
+++ b/xen/arch/x86/include/asm/x86_64/uaccess.h
@@ -26,15 +26,16 @@ void free_compat_arg_xlat(struct vcpu *v);
 #define xlat_page_start ((unsigned long)COMPAT_ARG_XLAT_VIRT_BASE)
 #define xlat_page_size  COMPAT_ARG_XLAT_SIZE
 #define xlat_page_left_size(xlat_page_current) \
-    (xlat_page_start + xlat_page_size - xlat_page_current)
+    (xlat_page_start + xlat_page_size - (xlat_page_current))
 
 #define xlat_malloc_init(xlat_page_current)    do { \
-    xlat_page_current = xlat_page_start; \
+    (xlat_page_current) = xlat_page_start; \
 } while (0)
 
 extern void *xlat_malloc(unsigned long *xlat_page_current, size_t size);
 
-#define xlat_malloc_array(_p, _t, _c) ((_t *) xlat_malloc(&_p, sizeof(_t) * _c))
+#define xlat_malloc_array(_p, _t, _c) ((_t *) xlat_malloc(&(_p), \
+                                                          sizeof(_t) * (_c)))
 
 /*
  * Valid if in +ve half of 48-bit address space, or above Xen-reserved area.
-- 
2.34.1

Re: [XEN PATCH 3/4] x86_64/uaccess: address violations of MISRA C Rule 20.7

[Stefano Stabellini]

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.
> 
> xlat_malloc_init is touched for consistency, despite the construct
> being already deviated.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 3/4] x86_64/uaccess: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 16.05.2024 01:19, 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.
>>
>> xlat_malloc_init is touched for consistency, despite the construct
>> being already deviated.
>>
>> No functional change.
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Acked-by: Jan Beulich <jbeulich@suse.com>

[XEN PATCH 4/4] x86_64/cpu_idle: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

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>
---
 xen/arch/x86/x86_64/cpu_idle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/cpu_idle.c b/xen/arch/x86/x86_64/cpu_idle.c
index fcd6fc0fc212..cc9febc03d60 100644
--- a/xen/arch/x86/x86_64/cpu_idle.c
+++ b/xen/arch/x86/x86_64/cpu_idle.c
@@ -93,7 +93,7 @@ long compat_set_cx_pminfo(uint32_t acpi_id,
             return -EFAULT; \
         guest_from_compat_handle(states, (_s_)->states); \
 \
-        for ( i = 0; i < _s_->count; i++ ) \
+        for ( i = 0; i < (_s_)->count; i++ ) \
         { \
            if ( unlikely(copy_from_guest_offset(&state, states, i, 1)) ) \
                return -EFAULT; \
-- 
2.34.1

Re: [XEN PATCH 4/4] x86_64/cpu_idle: address violations of MISRA C Rule 20.7

[Stefano Stabellini]

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>

Re: [XEN PATCH 4/4] x86_64/cpu_idle: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 16.05.2024 01:19, 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>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[Jan Beulich]

Oleksii,

On 15.05.2024 09:34, Nicola Vetrini wrote:
> Hi all,
> 
> this series aims to refactor some macros that cause violations of MISRA C Rule
> 20.7 ("Expressions resulting from the expansion of macro parameters shall be
> enclosed in parentheses"). All the macros touched by these patches are in some
> way involved in violations, and the strategy adopted to bring them into
> compliance is to add parentheses around macro arguments where needed.
> 
> Nicola Vetrini (4):
>   x86/vpmu: address violations of MISRA C Rule 20.7
>   x86/hvm: address violations of MISRA C Rule 20.7
>   x86_64/uaccess: address violations of MISRA C Rule 20.7
>   x86_64/cpu_idle: address violations of MISRA C Rule 20.7

for 4.18 we took a relaxed approach towards (simple) changes for Misra purposes.
I wonder whether you mean to permit the same for 4.19, or whether series like
this one rather want/need delaying until after branching.

Jan

>  xen/arch/x86/cpu/vpmu_amd.c               | 4 ++--
>  xen/arch/x86/hvm/mtrr.c                   | 2 +-
>  xen/arch/x86/hvm/rtc.c                    | 2 +-
>  xen/arch/x86/include/asm/hvm/save.h       | 2 +-
>  xen/arch/x86/include/asm/x86_64/uaccess.h | 7 ++++---
>  xen/arch/x86/x86_64/cpu_idle.c            | 2 +-
>  6 files changed, 10 insertions(+), 9 deletions(-)
> 

Re: [XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[Oleksii K.]

On Wed, 2024-05-15 at 09:48 +0200, Jan Beulich wrote:
> Oleksii,
> 
> On 15.05.2024 09:34, Nicola Vetrini wrote:
> > Hi all,
> > 
> > this series aims to refactor some macros that cause violations of
> > MISRA C Rule
> > 20.7 ("Expressions resulting from the expansion of macro parameters
> > shall be
> > enclosed in parentheses"). All the macros touched by these patches
> > are in some
> > way involved in violations, and the strategy adopted to bring them
> > into
> > compliance is to add parentheses around macro arguments where
> > needed.
> > 
> > Nicola Vetrini (4):
> >   x86/vpmu: address violations of MISRA C Rule 20.7
> >   x86/hvm: address violations of MISRA C Rule 20.7
> >   x86_64/uaccess: address violations of MISRA C Rule 20.7
> >   x86_64/cpu_idle: address violations of MISRA C Rule 20.7
> 
> for 4.18 we took a relaxed approach towards (simple) changes for
> Misra purposes.
> I wonder whether you mean to permit the same for 4.19, or whether
> series like
> this one rather want/need delaying until after branching.
Lets follow the same approach for 4.19.

Sorry for delayed answer.

~ Oleksii

> 
> Jan
> 
> >  xen/arch/x86/cpu/vpmu_amd.c               | 4 ++--
> >  xen/arch/x86/hvm/mtrr.c                   | 2 +-
> >  xen/arch/x86/hvm/rtc.c                    | 2 +-
> >  xen/arch/x86/include/asm/hvm/save.h       | 2 +-
> >  xen/arch/x86/include/asm/x86_64/uaccess.h | 7 ++++---
> >  xen/arch/x86/x86_64/cpu_idle.c            | 2 +-
> >  6 files changed, 10 insertions(+), 9 deletions(-)
> > 
> 

Re: [XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[Jan Beulich]

On 16.05.2024 17:58, Oleksii K. wrote:
> On Wed, 2024-05-15 at 09:48 +0200, Jan Beulich wrote:
>> On 15.05.2024 09:34, Nicola Vetrini wrote:
>>> this series aims to refactor some macros that cause violations of
>>> MISRA C Rule
>>> 20.7 ("Expressions resulting from the expansion of macro parameters
>>> shall be
>>> enclosed in parentheses"). All the macros touched by these patches
>>> are in some
>>> way involved in violations, and the strategy adopted to bring them
>>> into
>>> compliance is to add parentheses around macro arguments where
>>> needed.
>>>
>>> Nicola Vetrini (4):
>>>   x86/vpmu: address violations of MISRA C Rule 20.7
>>>   x86/hvm: address violations of MISRA C Rule 20.7
>>>   x86_64/uaccess: address violations of MISRA C Rule 20.7
>>>   x86_64/cpu_idle: address violations of MISRA C Rule 20.7
>>
>> for 4.18 we took a relaxed approach towards (simple) changes for
>> Misra purposes.
>> I wonder whether you mean to permit the same for 4.19, or whether
>> series like
>> this one rather want/need delaying until after branching.
> Lets follow the same approach for 4.19.

Well, okay. But if you don't say now until when this is okay, you'll
need to announce the "stop" very prominently later on, so no-one
misses it.

Jan

Re: [XEN PATCH 0/4] address violations of MISRA C Rule 20.7

[Oleksii K.]

On Thu, 2024-05-16 at 18:08 +0200, Jan Beulich wrote:
> > > for 4.18 we took a relaxed approach towards (simple) changes for
> > > Misra purposes.
> > > I wonder whether you mean to permit the same for 4.19, or whether
> > > series like
> > > this one rather want/need delaying until after branching.
> > Lets follow the same approach for 4.19.
> 
> Well, okay. But if you don't say now until when this is okay, you'll
> need to announce the "stop" very prominently later on, so no-one
> misses it.
For me it is okay until we don't have Hard Code Release deadline.

~ Oleksii