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

[Julien Grall <julien.grall@xxxxxxx>]

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