Re: [Xen-devel] [RFC PATCH 0/4] Add missing default labels to switch statements

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 22 Feb 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 22/02/2019 21:58, Stefano Stabellini wrote:
> > On Fri, 22 Feb 2019, Andrew Cooper wrote:
> >> On 22/02/2019 21:00, Stefano Stabellini wrote:
> >>> On Fri, 22 Feb 2019, Julien Grall wrote:
> >>>>>>> BTW, I checked the series with -Wswitch-default:
> >>>>>>> -Wswitch-default
> >>>>>>> Warn whenever a switch statement does not have a default case.
> >>>>>>>> Furthermore, using BUG() is a pretty bad idea in switch.
> >>>>>>> It is and not only in the switch. The reason I put BUG is that I tried
> >>>>>>> to follow
> >>>>>>> the existing "error handling" at those places.
> >>>>>> It is not because BUG() is been used today in some places that we need 
> >>>>>> to
> >>>>>> continue to spread it.
> >>>>>>
> >>>>>>> Use of BUG() itself is another topic which will also need to be
> >>>>>>> addressed
> >>>>>> So we should not add more of them...
> >>>>> Again, I see this as a dedicated change. So, in the current series I 
> >>>>> think
> >>>>> it is
> >>>>> acceptable to use the existing way of error handling if any at all.
> >>>> That's not how it works in upstream. If you know some constructs are 
> >>>> wrong, it
> >>>> is best to try to address partially the problem directly then having so 
> >>>> you
> >>>> reduce the amounts of change afterwards.
> >>>>
> >>>> So please try to not introduce more BUG() in the code base.
> >>> Hi Oleksandr, Julien,
> >>>
> >>> Julien's right that we should not introduce any more BUG()s. In fact,
> >>> each of them makes the code less safe, not more safe! The purpose of
> >>> MISRAC 16.4 is "defensive programming": write the code in a way that is
> >>> more (not less!) resilient to failure.
> >>>
> >>> So, I think it is a good idea to introduce a default label because it
> >>> can help us spot unexpected issues. Instead of calling BUG() in the
> >>> default handler, which is detrimental, we should return an error when
> >>> possible, or just print a warning.
> >>
> >> domain_crash() is almost always better than BUG().  It is very obvious
> >> if it gets hit, and wont crash Xen.
> > 
> > That's a good suggestion.
> > 
> > 
> >>> As 16.4 clearly state, even a simple comment would be enough to address
> >>> the rule. We just need to explain why a default label is not needed.
> >>> Such as:
> >>>
> >>>    default:
> >>>    /* unreachable because blah and blah */
> >>
> >> What a simple comment doesn't do is avoid breaking -Wswitch.
> > 
> > I don't know how to reconcile 16.4 with -Wswitch. One could argue that
> > -Wswitch could be a good way to address 16.4, but then we introduce a
> > compiler specific requirement. Typically gcc is not the compiler of
> > choice for these environments, unfortunately forcing gcc is not an
> > option.
> 
> Well, you could build with GCC and then build with your custom 
> compiler... 

This suggestion is problematic: as an individual interested in MISRA-C
compliance, I only have the MISRA-C rules in my hands. I don't know how
to deal with suggestions like this one, that don't comply to the Rules,
but it tries to address the same issue in a different manner.

I cannot rule out that it wouldn't work, but also I cannot be sure that
it would work. In short, I have no way to make progress or to find out
how to move forward. I guess as a contributor I would be forced to go
back to the MISRAC compliance experts and ask for their opinion. (One
non-technical issue is who is going to pay them for spending their time
on this.) But what if they say it is not acceptable for compliance?

This is a great topic to discuss in March and decide what to do in these
situations.


> But, GCC is pretty much the only choice for Xen on Arm today 
> as we don't build with clang and I pretty doubt we can build with compcert.

Obviously, this has to change if we want to make progress on safety
certifications.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel