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

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

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@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxx>
> > Signed-off-by: Simone Ballarin  <simone.ballarin@xxxxxxxxxxx>
> > ---
> >   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)