Re: [XEN PATCH 03/13] xen/arm: address violations of MISRA C:2012 Directive 4.10

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

On 30/08/23 15:01, Jan Beulich wrote:
> On 30.08.2023 14:53, Simone Ballarin wrote:
> > On 29/08/23 00:10, Julien Grall wrote:
> > > On Mon, 28 Aug 2023 at 09:20, Simone Ballarin <simone.ballarin@xxxxxxxxxxx>
> > > wrote:
> > > > --- a/xen/arch/arm/include/asm/hypercall.h
> > > > +++ b/xen/arch/arm/include/asm/hypercall.h
> > > > @@ -1,10 +1,10 @@
> > > > +#ifndef __ASM_ARM_HYPERCALL_H__
> > > > +#define __ASM_ARM_HYPERCALL_H__
> > > > +
> > > >    #ifndef __XEN_HYPERCALL_H__
> > > >    #error "asm/hypercall.h should not be included directly - include
> > > > xen/hypercall.h instead"
> > > >    #endif
> > > >
> > > > -#ifndef __ASM_ARM_HYPERCALL_H__
> > > > -#define __ASM_ARM_HYPERCALL_H__
> > > > -
> > >
> > > I understand that you are trying to fix a misra violation. However, this
> > > feels like it was done on purpose.
> > >
> > > With the new change, you would not always check that the file were included
> > > at the correct place. I am not against this change but this ought to be
> > > explained.
> > I don't think the semantics have changed. Please correct me if I'm wrong.
> >
> > With this change, the only situation where the check is not performed is
> > when __ASM_ARM_HYPERCALL_H__ is defined (i.e. the file has already been
> > successfully included). This implies that if __ASM_ARM_HYPERCALL_H__ is
> > defined, then __XEN_HYPERCALL_H__ is also defined, so the check would be
> > useless.
> >
> > The same thing happened with the code before the change: if I include
> > the file after xen/hypercall.h, the check will always succeed.
> Hmm, I think you're right, but I draw a different conclusion: The check
> fails to work as intended. And this can only be repaired without your
> adjustment.
Ok, I will just deviate these cases.

> Jan
--
Simone Ballarin, M.Sc.

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