Re: [XEN PATCH v2] x86: p2m-pod: address violation of MISRA C Rule 2.1

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-07-01 10:36, Jan Beulich wrote:
> On 28.06.2024 08:30, Nicola Vetrini wrote:
> > The label 'out_unmap' is only reachable after ASSERT_UNREACHABLE,
> > so the code below is only executed upon erroneously reaching that
> > program point and calling domain_crash, thus resulting in the
> > for loop after 'out_unmap' to become unreachable in some 
> > configurations.
> First: As you have come to be used to, briefly stating the rule itself
> (rather than just its number, requiring people like me - who have not
> memorized all the rule numbers - to go look up what rule this is) would
> be nice.
Sure

> This being about unreachable code, why are the domain_crash() not the
> crucial points of "unreachability"? And even if they weren't there, why
> wouldn't it be the goto or ...
>
> > --- a/xen/arch/x86/mm/p2m-pod.c
> > +++ b/xen/arch/x86/mm/p2m-pod.c
> > @@ -1040,6 +1040,7 @@ out_unmap:
> >       * Something went wrong, probably crashing the domain.  Unmap
> >       * everything and return.
> >       */
> > +    /* SAF-8-safe Rule 2.1: defensive programming */
> >      for ( i = 0; i < count; i++ )
> >          if ( map[i] )
> >              unmap_domain_page(map[i]);
> ... the label (just out of context) where the comment needs to go?
Because of the way this rule's configuration work, deviations are placed 
on the construct that ends up being the target of the unreachability, 
rather than (one of) the causes of such unreachability. Putting the 
comment on the label works for ECLAIR by offsetting its target 
statement, but not for other tools afaik.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)