[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 Stefano,

On 25/02/2019 17:47, Stefano Stabellini wrote:
On Mon, 25 Feb 2019, George Dunlap wrote:
On Mon, Feb 25, 2019 at 11:42 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:

On 22.02.19 at 22:33, <andrew.cooper3@xxxxxxxxxx> wrote:
P.S. There is a solution here which could work, but IMO a better use of
time and energy would be to get MISRA to update their rules to match
this century, and stop getting in the way of compiler features intended
to help the programmer avoid bugs.

As much as I'm with you in desiring the compiler aid given to not get
undermined, I think this MISRA rule isn't in need of modernizing: It's
one thing for the compiler to help with in-range enumerators, and it's
another to demand that unintentional out-of-range ones don't cause
actual harm (like crashing your car into the next tree). This is even
more so that iirc there's no warning if you pass a plain integer into a
function whose parameter specifies an enum, or if you assign a plain
integer to an enum types variable.

FWIW I was thinking the same thing.

Jan described exactly the scenario Rule 16.4 attempts to protect us
from. FYI MISRAC explains the reasons behind each rule, this is why I
think it would be great for you (you as in the x86 and Arm maintainers
mainly) to have access to it so you can read the explanation by
yourselves.

Also, all the alternative suggestions about using complier features to
check switch statements are based on the fact that one can use those
compilers. I don't think we can/should/want to mandate which compilers
are used with the Xen codebase. We don't want to get into the situation
where the code is "safe" only when it is built with gcc.

The compiler is not here to make it "safe", but helps you to find place where we forgot to handle a case when extending an enum.

Lets imagine for a moment that we decide to extend p2m_type_t. There are quite a few switch using this enum. You will no deny that finding out all the missing path during reviewing can be tricky. Here the compiler give you a bit of help to find to check all the places were handled.

Without the compiler aid, you may have to wait until someone hit the error path before actually discovering the missing bits. This may take quite sometimes (more than a release) depending on how much the code is exercised and potentially a pain to debug.

Regardless what the safety-certification folks are going to use, GCC is likely going to stay one of the main compiler used to build Xen. We are already struggling to keep up with the review, so I fail to see why we should make reviewer's life a bit more miserable by preventing us using "safety" options from GCC.

So I would like to find a way to keep that benefits for reviewers while also addressing MISRA rules. I suggested a GCC flag (-Wswitch-enum) on another e-mail [1] but Jan was not against such use.

I don't have a better suggestion so far.

Cheers,

[1] <5C73DB260200007800219CF1@xxxxxxxxxxxxxxxxxxxxxxxx>

--
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®.