[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:27 PM, Julien Grall wrote:
Hi Oleksandr,

On 22/02/2019 11:13, Oleksandr Andrushchenko wrote:
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.

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.
The answer is that what happens if we by any reason either by a mistake or any other mean have a build which doesn't have -Wswitch or -Wall. I mean that everything changes and having "default" in the source does guarantee the handling as it was intended to be.

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.

But what happens if you miss default handling and because of bugs in the code you do not
handle "compile time unexpected values"?

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.

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.


Xen-devel mailing list



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