Re: [XEN PATCH v2 09/13] x86/mm: add defensive return

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 24/06/24 17:39, Jan Beulich wrote:
> On 24.06.2024 11:04, Federico Serafini wrote:
> > Add defensive return statement at the end of an unreachable
> > default case. Other than improve safety,
> It is particularly with this in mind that ...
>
> > this meets the requirements
> > to deviate a violation of MISRA C Rule 16.3: "An unconditional `break'
> > statement shall terminate every switch-clause".
> >
> > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > ---
> >   xen/arch/x86/mm.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> > index 648d6dd475..2e19ced15e 100644
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -916,6 +916,7 @@ get_page_from_l1e(
> >                   return 0;
> >               default:
> >                   ASSERT_UNREACHABLE();
> > +                return 0;
> >               }
> ... returning "success" in such a case can't be quite right. As indicated
> elsewhere, really we want -EINTERNAL for such cases, just that errno.h
> doesn't know anything like this, and I'm also unaware of a somewhat
> similar identifier that we might "clone" from elsewhere. Hence we'll want
> to settle on some existing error code which we then use here and in
> similar situations. EINVAL is the primary example of what I would prefer
> to _not_ see used for this.
Sorry, I got confused by the ASSERT_UNREACHABLE followed by a
return 0 few lines above.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)