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

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

On 2/22/19 11:33 PM, Andrew Cooper 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
> > > > > > Warn whenever a switch statement does not have a default case.
Will you be ok to turn this particular switch on by default?
Or you suggest that anyone interested to do so on their own will?
If we want to turn many more warnings a bit later?
> > > > > > > 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.
> > > That's not how it works in upstream. If you know some constructs are wrong, it
> > > is best to try to address partially the problem directly then having so you
> > > reduce the amounts of change afterwards.
So, then we need to get rid of BUG() from the existing code first,
not trying to solve two issues at a time: rule 16.4 and BUG().
I still do think these two changes are not strictly relevant,
but 16.4 just suffers from consequences of BUG() being used.
> > > So please try to not introduce more BUG() in the code base.
> > Hi Oleksandr, Julien,
> >
> > Julien's right that we should not introduce any more BUG()s. In fact,
> > each of them makes the code less safe, not more safe! The purpose of
> > MISRAC 16.4 is "defensive programming": write the code in a way that is
> > more (not less!) resilient to failure.
> >
> > So, I think it is a good idea to introduce a default label because it
> > can help us spot unexpected issues. Instead of calling BUG() in the
> > default handler, which is detrimental, we should return an error when
> > possible, or just print a warning.
> domain_crash() is almost always better than BUG().  It is very obvious
> if it gets hit, and wont crash Xen.
Thank you for suggestions
> > 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 */
This is true...
> What a simple comment doesn't do is avoid breaking -Wswitch.
... as well as this comment as well.
> This requirement is actively hostile towards compilers trying to help
> you spot when you made a mistake and forgot to update one of the $N
> places you needed to.
This is a trade-off: if your compiler is buggy or not supporting
the switches then the code itself defends from such cases, hence
the "default".
> In this case, I don't think "Because MISRA demand it" is a good enough
> justification to offset the increased error-prone-ness of the result.
...and if you forget to enable -Wswitch or use a compiler that
doesn't support this switch then what?
> ~Andrew
>
> 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.
Well, I am a little bit pessimistic here. The same suggestion
MISRA guys may have: go change your code as the rules we have
are widely accepted and the code which wants to comply must obey the
rules. What is more is that if MISRA agrees that compiler switches can
be used, then what are those compilers? I can imagine that various versions
of even gcc might not support all the switches we want. So what happens
next? We dictate gcc guys to make sure to support all the switches that we
agreed with MISRA?


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel