[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.