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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Oleksandr,

On 22/02/2019 11:13, Oleksandr Andrushchenko wrote:
> 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.
You have a point for the integer switch.

> What if with time enumeration changes, what if some of the case statements get 
> removed and so on.
In that case, the compiler will throw an error (Xen is built with -Werror). So 
for enumeration the compiler will help us to spot the missing places.
If you add a default case, you remove us a good way to check we actually add the 
new element everywhere.
> 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.
It does not mean this is always the best solution. If we take the example of the 
enumeration, now you can't easily detect at compilation time you missed a case. 
This could potentially lead to painful debugging session to understand what is 
missing.
> 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...

Cheers,

--
Julien Grall

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