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

On 25/02/2019 10:11, Oleksandr Andrushchenko wrote:
On 2/23/19 12:08 AM, Julien Grall wrote:
On 22/02/2019 21:00, Stefano Stabellini wrote:
On Fri, 22 Feb 2019, Julien Grall wrote:
BTW, I checked the series with -Wswitch-default:
-Wswitch-default
As 16.4 clearly state, even a simple comment would be enough to address
the rule. We just need to explain why a default label is not needed.
Such as:

    default:
    /* unreachable because blah and blah */
... as we already have defensive code. Indeed, in most of the switch we
deal with potential issue by initializing the variable before the
switch. If you look at the first patch, a lot of "default: break;" is
introduced. So what's our benefits? How this code is more defensive than
what we currently have?
...and if you remove those initialized variables and forget
to update the switch with default? Or change the initial value
the way your previous switch assumptions do not work anymore?
I am not trying to defend MISRA here, but things change and we all
make mistakes. This is why defensive programming

I am not totally against using default label. However, how is the following code is defensive?

default:
   break;

This heavily relies on the code surrounding if dealing correctly with the error and will never change. It feels to me we are only trying to make MISRA happy.

If we were writing defensive code, then you would definitely not have empty default and would at least contain an ASSERT_UNREACHABLE or error handling.

But then, you still rely on the default case to do the right thing if there was an error. I fail to understand how this is better than what we currently have.

Furthermore, how this is going to help us (thanks to -Wswitch) if an
enumerate is extended and we miss a case that the compiler don't notice
anymore?
And if the switch's index is not an enumeration, but a plain integer...?

We have to differentiate enum vs plain integer.

For enum, a compiler supporting -Wswitch can helps us to catch when we forgot to handle a element of the enum. The default case defeats -Wswitch. Yet having a default could protect us against code error. It looks like that GCC have an option -Wswitch-enum that could help us. The option will similar to -Wswitch except it will warn even when there are a default case. I am curious to see if we can use it reliably in Xen.

For plain integer, we indeed need to have a *sensible* default case.

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