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

Simone Ballarin posted 1 patch 4 months, 4 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/a295b44a9fbb02f962720e086588437876e02ce3.1701430079.git.maria.celeste.cesario@bugseng.com
There is a newer version of this series
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   | 3 ++-
4 files changed, 5 insertions(+), 4 deletions(-)
[PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Simone Ballarin 4 months, 4 weeks ago
From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>

Remove or amend casts to comply with Rule 11.8.

The violations are resolved either by adding missing const
qualifiers in casts or 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 change.

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

Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
---
 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   | 3 ++-
 4 files changed, 5 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..b28eb5de7a 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 = (const uintptr_t)guest_cpu_user_regs() - (const 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..64f1e0d400 100644
--- a/xen/arch/x86/include/asm/regs.h
+++ b/xen/arch/x86/include/asm/regs.h
@@ -6,7 +6,8 @@
 
 #define guest_mode(r)                                                         \
 ({                                                                            \
-    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
+    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() -             \
+                                                        (const 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 v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Julien Grall 4 months, 4 weeks ago
Hi Simone,

On 01/12/2023 11:37, Simone Ballarin wrote:
> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
> 
> Remove or amend casts to comply with Rule 11.8.
> 
> The violations are resolved either by adding missing const
> qualifiers in casts or 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 change.
> 
> ---
> 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.
> 
> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
> ---
>   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   | 3 ++-
>   4 files changed, 5 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..b28eb5de7a 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 = (const uintptr_t)guest_cpu_user_regs() - (const uintptr_t)(r);

NIT: The const should not be necessary here. Am I correct?

>       /* 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..64f1e0d400 100644
> --- a/xen/arch/x86/include/asm/regs.h
> +++ b/xen/arch/x86/include/asm/regs.h
> @@ -6,7 +6,8 @@
>   
>   #define guest_mode(r)                                                         \
>   ({                                                                            \
> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
> +    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() -             \
> +                                                        (const uintptr_t(r)); \

Was this compiled on x86? Shouldn't the last one be (const uintptr_t)(r))?

Cheers,

-- 
Julien Grall
Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Simone Ballarin 4 months, 4 weeks ago
On 01/12/23 12:48, Julien Grall wrote:
> Hi Simone,
> 
> On 01/12/2023 11:37, Simone Ballarin wrote:
>> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
>>
>> Remove or amend casts to comply with Rule 11.8.
>>
>> The violations are resolved either by adding missing const
>> qualifiers in casts or 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 change.
>>
>> ---
>> 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.
>>
>> Signed-off-by: Maria Celeste Cesario  <maria.celeste.cesario@bugseng.com>
>> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
>> ---
>>   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   | 3 ++-
>>   4 files changed, 5 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..b28eb5de7a 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 = (const uintptr_t)guest_cpu_user_regs() - 
>> (const uintptr_t)(r);
> 
> NIT: The const should not be necessary here. Am I correct?

The const in the first parameter is not necessary, I will drop it.

> 
>>       /* 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..64f1e0d400 100644
>> --- a/xen/arch/x86/include/asm/regs.h
>> +++ b/xen/arch/x86/include/asm/regs.h
>> @@ -6,7 +6,8 @@
>>   #define 
>> guest_mode(r)                                                         \
>>   
>> ({                                                                            \
>> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char 
>> *)(r);         \
>> +    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() 
>> -             \
>> +                                                        (const 
>> uintptr_t(r)); \
> 
> Was this compiled on x86? Shouldn't the last one be (const uintptr_t)(r))?
> 

Yes, you are right. I'll fix in it in v3.

> Cheers,
> 

-- 
Simone Ballarin, M.Sc.

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


Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Julien Grall 4 months, 4 weeks ago

On 01/12/2023 13:42, Simone Ballarin wrote:
> On 01/12/23 12:48, Julien Grall wrote:
>> Hi Simone,
>>
>> On 01/12/2023 11:37, Simone Ballarin wrote:
>>> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
>>>
>>> Remove or amend casts to comply with Rule 11.8.
>>>
>>> The violations are resolved either by adding missing const
>>> qualifiers in casts or 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 change.
>>>
>>> ---
>>> 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.
>>>
>>> Signed-off-by: Maria Celeste Cesario  
>>> <maria.celeste.cesario@bugseng.com>
>>> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
>>> ---
>>>   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   | 3 ++-
>>>   4 files changed, 5 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..b28eb5de7a 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 = (const uintptr_t)guest_cpu_user_regs() - 
>>> (const uintptr_t)(r);
>>
>> NIT: The const should not be necessary here. Am I correct?
> 
> The const in the first parameter is not necessary, I will drop it.

I am confused. In the case of 'r' the const applied to the pointee not 
the pointer (e.g. the pointer can be modified but not the content). So 
the 'const' should not be necessary even for the second parameter.

Cheers,

-- 
Julien Grall

Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Simone Ballarin 4 months, 4 weeks ago
On 01/12/23 14:48, Julien Grall wrote:
> 
> 
> On 01/12/2023 13:42, Simone Ballarin wrote:
>> On 01/12/23 12:48, Julien Grall wrote:
>>> Hi Simone,
>>>
>>> On 01/12/2023 11:37, Simone Ballarin wrote:
>>>> From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
>>>>
>>>> Remove or amend casts to comply with Rule 11.8.
>>>>
>>>> The violations are resolved either by adding missing const
>>>> qualifiers in casts or 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 change.
>>>>
>>>> ---
>>>> 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.
>>>>
>>>> Signed-off-by: Maria Celeste Cesario 
>>>> <maria.celeste.cesario@bugseng.com>
>>>> Signed-off-by: Simone Ballarin  <simone.ballarin@bugseng.com>
>>>> ---
>>>>   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   | 3 ++-
>>>>   4 files changed, 5 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..b28eb5de7a 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 = (const uintptr_t)guest_cpu_user_regs() - 
>>>> (const uintptr_t)(r);
>>>
>>> NIT: The const should not be necessary here. Am I correct?
>>
>> The const in the first parameter is not necessary, I will drop it.
> 
> I am confused. In the case of 'r' the const applied to the pointee not 
> the pointer (e.g. the pointer can be modified but not the content). So 
> the 'const' should not be necessary even for the second parameter.
> 

Yes, sorry. Here there is no reason to use a const: if we cast to a 
non-pointer type (uintptr_t) rule 11.8 does not apply.

> Cheers,
> 

-- 
Simone Ballarin, M.Sc.

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


Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Jan Beulich 4 months, 4 weeks ago
On 01.12.2023 12:48, Julien Grall wrote:
> On 01/12/2023 11:37, Simone Ballarin wrote:
>> --- 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 = (const uintptr_t)guest_cpu_user_regs() - (const uintptr_t)(r);
> 
> NIT: The const should not be necessary here. Am I correct?
> 
>> --- a/xen/arch/x86/include/asm/regs.h
>> +++ b/xen/arch/x86/include/asm/regs.h
>> @@ -6,7 +6,8 @@
>>   
>>   #define guest_mode(r)                                                         \
>>   ({                                                                            \
>> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
>> +    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() -             \
>> +                                                        (const uintptr_t(r)); \
> 
> Was this compiled on x86? Shouldn't the last one be (const uintptr_t)(r))?

And again with the stray const-s dropped and with indentation adjusted.

Jan
Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Simone Ballarin 4 months, 4 weeks ago
On 01/12/23 14:03, Jan Beulich wrote:
> On 01.12.2023 12:48, Julien Grall wrote:
>> On 01/12/2023 11:37, Simone Ballarin wrote:
>>> --- 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 = (const uintptr_t)guest_cpu_user_regs() - (const uintptr_t)(r);
>>
>> NIT: The const should not be necessary here. Am I correct?
>>
>>> --- a/xen/arch/x86/include/asm/regs.h
>>> +++ b/xen/arch/x86/include/asm/regs.h
>>> @@ -6,7 +6,8 @@
>>>    
>>>    #define guest_mode(r)                                                         \
>>>    ({                                                                            \
>>> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
>>> +    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() -             \
>>> +                                                        (const uintptr_t(r)); \
>>
>> Was this compiled on x86? Shouldn't the last one be (const uintptr_t)(r))?
> 
> And again with the stray const-s dropped and with indentation adjusted.
> 

I will remove the const in the first parameter and fix the indentation
in the following way:

unsigned long diff = (uintptr_t)guest_cpu_user_regs() -                \
                      (const uintptr_t)(r);                             \

> Jan
> 

-- 
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)
Re: [PATCH v2] xen: address violations of MISRA C:2012 Rule 11.8.
Posted by Jan Beulich 4 months, 3 weeks ago
On 01.12.2023 14:44, Simone Ballarin wrote:
> On 01/12/23 14:03, Jan Beulich wrote:
>> On 01.12.2023 12:48, Julien Grall wrote:
>>> On 01/12/2023 11:37, Simone Ballarin wrote:
>>>> --- 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 = (const uintptr_t)guest_cpu_user_regs() - (const uintptr_t)(r);
>>>
>>> NIT: The const should not be necessary here. Am I correct?
>>>
>>>> --- a/xen/arch/x86/include/asm/regs.h
>>>> +++ b/xen/arch/x86/include/asm/regs.h
>>>> @@ -6,7 +6,8 @@
>>>>    
>>>>    #define guest_mode(r)                                                         \
>>>>    ({                                                                            \
>>>> -    unsigned long diff = (char *)guest_cpu_user_regs() - (char *)(r);         \
>>>> +    unsigned long diff = (const uintptr_t)guest_cpu_user_regs() -             \
>>>> +                                                        (const uintptr_t(r)); \
>>>
>>> Was this compiled on x86? Shouldn't the last one be (const uintptr_t)(r))?
>>
>> And again with the stray const-s dropped and with indentation adjusted.
>>
> 
> I will remove the const in the first parameter and fix the indentation
> in the following way:
> 
> unsigned long diff = (uintptr_t)guest_cpu_user_regs() -                \
>                       (const uintptr_t)(r);                             \

That still looks to be one off, but (supported by the \ placement) possibly
merely an artifact of how your or my mail client is configured. It looks
right at https://lists.xen.org/archives/html/xen-devel/2023-12/msg00057.html.

Jan