[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

On 2/22/19 1:05 PM, Julien Grall wrote:

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:
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:
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


Thank you,

Xen-devel mailing list



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