[PATCH v3] xen: address violations of MISRA C:2012 Rule 11.8.

[PATCH v3] xen: address violations of MISRA C:2012 Rule 11.8.

[Simone Ballarin]

From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>

Remove or amend casts to comply with Rule 11.8.

Fix violations by adding missing const qualifier in cast.
Fix violations by removing unnecessary cast.
Change type of operands from char* to uintptr_t: uintptr_t is
the appropriate type for memory address operations.

No functional changes.

Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>

---
Changes in v3:
- drop const qualifier in both operands of the variable diff.
- fix parentheses error.
Changes in v2:
- arm/regs.h: add const qualifier to the first operand,
    change types of both operands from char* to uintptr_t.
- x86/regs.h: add const qualifier to both operands. Change
    types of both operands from char* to uintptr_t to
    conform with the arm version.
- dom0less-build.c: rebase change in the new file.
---
 xen/arch/arm/dom0less-build.c     | 2 +-
 xen/arch/arm/include/asm/atomic.h | 2 +-
 xen/arch/arm/include/asm/regs.h   | 2 +-
 xen/arch/x86/include/asm/regs.h   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index d39cbd969a..fb63ec6fd1 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -354,7 +354,7 @@ static int __init handle_passthrough_prop(struct kernel_info *kinfo,
     if ( node == NULL )
     {
         printk(XENLOG_ERR "Couldn't find node %s in host_dt!\n",
-               (char *)xen_path->data);
+               xen_path->data);
         return -EINVAL;
     }
 
diff --git a/xen/arch/arm/include/asm/atomic.h b/xen/arch/arm/include/asm/atomic.h
index 64314d59b3..517216d2a8 100644
--- a/xen/arch/arm/include/asm/atomic.h
+++ b/xen/arch/arm/include/asm/atomic.h
@@ -154,7 +154,7 @@ static always_inline void write_atomic_size(volatile void *p,
  */
 static inline int atomic_read(const atomic_t *v)
 {
-    return *(volatile int *)&v->counter;
+    return *(const volatile int *)&v->counter;
 }
 
 static inline int _atomic_read(atomic_t v)
diff --git a/xen/arch/arm/include/asm/regs.h b/xen/arch/arm/include/asm/regs.h
index 8a0db95415..f998aedff5 100644
--- a/xen/arch/arm/include/asm/regs.h
+++ b/xen/arch/arm/include/asm/regs.h
@@ -48,7 +48,7 @@ static inline bool regs_mode_is_32bit(const struct cpu_user_regs *regs)
 
 static inline bool guest_mode(const struct cpu_user_regs *r)
 {
-    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);
+    unsigned long diff = (uintptr_t)guest_cpu_user_regs() - (uintptr_t)(r);
     /* Frame pointer must point into current CPU stack. */
     ASSERT(diff < STACK_SIZE);
     /* If not a guest frame, it must be a hypervisor frame. */
diff --git a/xen/arch/x86/include/asm/regs.h b/xen/arch/x86/include/asm/regs.h
index 3fb94deedc..ddf5e14e57 100644
--- a/xen/arch/x86/include/asm/regs.h
+++ b/xen/arch/x86/include/asm/regs.h
@@ -6,7 +6,7 @@
 
 #define guest_mode(r)                                                         \
 ({                                                                            \
-    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
+    unsigned long diff = (uintptr_t)guest_cpu_user_regs() - (uintptr_t)(r);   \
     /* Frame pointer must point into current CPU stack. */                    \
     ASSERT(diff < STACK_SIZE);                                                \
     /* If not a guest frame, it must be a hypervisor frame. */                \
-- 
2.40.0

Re: [PATCH v3] xen: address violations of MISRA C:2012 Rule 11.8.

[Jan Beulich]

On 04.12.2023 17:32, Simone Ballarin wrote:
> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
> 
> Remove or amend casts to comply with Rule 11.8.
> 
> Fix violations by adding missing const qualifier in cast.
> Fix violations by removing unnecessary cast.
> Change type of operands from char* to uintptr_t: uintptr_t is
> the appropriate type for memory address operations.
> 
> No functional changes.
> 
> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>

I consider it good practice to at least briefly say what the rule is
about, so it is clear why certain changes need doing.

> --- a/xen/arch/arm/include/asm/atomic.h
> +++ b/xen/arch/arm/include/asm/atomic.h
> @@ -154,7 +154,7 @@ static always_inline void write_atomic_size(volatile void *p,
>   */
>  static inline int atomic_read(const atomic_t *v)
>  {
> -    return *(volatile int *)&v->counter;
> +    return *(const volatile int *)&v->counter;
>  }

What about PPC's identical code?

> --- a/xen/arch/x86/include/asm/regs.h
> +++ b/xen/arch/x86/include/asm/regs.h
> @@ -6,7 +6,7 @@
>  
>  #define guest_mode(r)                                                         \
>  ({                                                                            \
> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
> +    unsigned long diff = (uintptr_t)guest_cpu_user_regs() - (uintptr_t)(r);   \
>      /* Frame pointer must point into current CPU stack. */                    \
>      ASSERT(diff < STACK_SIZE);                                                \
>      /* If not a guest frame, it must be a hypervisor frame. */                \

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

Jan

Re: [PATCH v3] xen: address violations of MISRA C:2012 Rule 11.8.

[Simone Ballarin]

On 04/12/23 17:54, Jan Beulich wrote:
> On 04.12.2023 17:32, Simone Ballarin wrote:
>> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
>>
>> Remove or amend casts to comply with Rule 11.8.
>>
>> Fix violations by adding missing const qualifier in cast.
>> Fix violations by removing unnecessary cast.
>> Change type of operands from char* to uintptr_t: uintptr_t is
>> the appropriate type for memory address operations.
>>
>> No functional changes.
>>
>> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
>> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
> 
> I consider it good practice to at least briefly say what the rule is
> about, so it is clear why certain changes need doing.
> 
>> --- a/xen/arch/arm/include/asm/atomic.h
>> +++ b/xen/arch/arm/include/asm/atomic.h
>> @@ -154,7 +154,7 @@ static always_inline void write_atomic_size(volatile void *p,
>>    */
>>   static inline int atomic_read(const atomic_t *v)
>>   {
>> -    return *(volatile int *)&v->counter;
>> +    return *(const volatile int *)&v->counter;
>>   }
> 
> What about PPC's identical code?

If the ARM part is accepted, I will include this change in an upcoming 
series for Rule 11.8 (which I'm working on), otherwise in v4.

> 
>> --- a/xen/arch/x86/include/asm/regs.h
>> +++ b/xen/arch/x86/include/asm/regs.h
>> @@ -6,7 +6,7 @@
>>   
>>   #define guest_mode(r)                                                         \
>>   ({                                                                            \
>> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
>> +    unsigned long diff = (uintptr_t)guest_cpu_user_regs() - (uintptr_t)(r);   \
>>       /* Frame pointer must point into current CPU stack. */                    \
>>       ASSERT(diff < STACK_SIZE);                                                \
>>       /* If not a guest frame, it must be a hypervisor frame. */                \
> 
> This part
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> Jan

-- 
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)

Re: [PATCH v3] xen: address violations of MISRA C:2012 Rule 11.8.

[Julien Grall]

On 04/12/2023 16:32, Simone Ballarin wrote:
> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
> 
> Remove or amend casts to comply with Rule 11.8.
> 
> Fix violations by adding missing const qualifier in cast.
> Fix violations by removing unnecessary cast.
> Change type of operands from char* to uintptr_t: uintptr_t is
> the appropriate type for memory address operations.
> 
> No functional changes.
> 
> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

-- 
Julien Grall