Re: [XEN PATCH 11/11] x86/mm: Add assertion to address MISRA C:2012 Rule 2.1

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 03/08/2023 11:20, Jan Beulich wrote:
> On 02.08.2023 16:38, Nicola Vetrini wrote:
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -4879,6 +4879,7 @@ long arch_memory_op(unsigned long cmd, 
> > XEN_GUEST_HANDLE_PARAM(void) arg)
> >          return subarch_memory_op(cmd, arg);
> >      }
> >
> > +    ASSERT_UNREACHABLE();
> >      return 0;
> >  }
> I'd prefer to instead switch earlier "return 0" to "break".
Ok

> > --- a/xen/arch/x86/mm/p2m-pod.c
> > +++ b/xen/arch/x86/mm/p2m-pod.c
> > @@ -1045,6 +1045,7 @@ p2m_pod_zero_check(struct p2m_domain *p2m, const 
> > gfn_t *gfns, unsigned int count
> >      }
> >
> >      return;
> > +    ASSERT_UNREACHABLE();
> >
> >  out_unmap:
> >      /*
> In the description you say "before", but here you add something _after_
> "return". What's the deal?
>
> Jan
In this case the unreachable part is that after the label (looking at it 
now, I should have
put the assert after the label to make it clear), because earlier all 
jumps to
'out_unmap' are like this:

  ASSERT_UNREACHABLE();
  domain_crash(d);
  goto out_unmap;

As I understood it, this is a defensive coding measure, preventing pages 
to remain mapped if,
for some reason the above code actually executes. Am I correct?

Regards,

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)