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

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

On 2/22/19 1:05 PM, Julien Grall wrote:
> Hi,
>
> On 22/02/2019 10:27, Andrew Cooper wrote:
> > On 22/02/2019 09:57, Oleksandr Andrushchenko wrote:
> > > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> > >
> > > Hello, everybody!
> > >
> > > We at EPAM Systems would like to present first series of patches 
> > > targeting Xen
> > > on ARM Functional Safety certification (ISO61508 based): 
> > > implementation of
> > > MISRA [1] C:2012 Rule 16.4 which requires that every switch 
> > > statement has a
> > > default label as a measure of defensive programming technique.
> > Hang on - what?
> >
> > Can someone attempt to justify why actively breaking -Wswitch is going
> > to result in safer/better code?
> I was about to ask the same. There are quite a few cases where this 
> series is going to make more difficult extending enum.
Well, I am not sure I can truly defend MISRA requirements here, but I'll try
to express my view on that.

Let us have a look at gcc's options [1]: so, for what you are saying:
"-Wswitch
Warn whenever a switch statement has an index of enumerated type and 
lacks a case for one or more of the named codes of that enumeration. 
(The presence of a default label prevents this warning.) case labels 
outside the enumeration range also provoke warnings when this option is 
used (even if there is a default label). This warning is enabled by -Wall."
So, if we do not have all the cases covered then this compiler's switch 
will fire
a warning (error). What if we have an integer as a switch's index, not 
an enumeration,
so then there is no easy way to handle all the cases and we have to 
provide *default* statement.
What if with time enumeration changes, what if some of the case 
statements get removed and so on.
All these, as per my understanding, lead to a defensive programming 
approach, e.g. do not rely on what
is here at the moment, but think that everything can change any time soon.

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.
Use of BUG() itself is another topic which will also need to be addressed
> A guest would be able to crash the whole platform if there was a 
> coding mistake. Instead we should use ASSERT_UNREACHABLE() and provide 
> proper fallback whenever it is possible.
Exactly, as I said we have to get rid of BUG() and friends and handle 
errors instead
> Cheers,
Thank you,
Oleksandr

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