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

[Julien Grall <Julien.Grall@xxxxxxx>]

Hi,

On 22/02/2019 22:34, Stefano Stabellini wrote:
> On Fri, 22 Feb 2019, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 22/02/2019 21:58, Stefano Stabellini wrote:
>>> On Fri, 22 Feb 2019, 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.
>>>>>>>>>> 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 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.
>>>
>>> That's a good suggestion.
>>>
>>>
>>>>> 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 */
>>>>
>>>> What a simple comment doesn't do is avoid breaking -Wswitch.
>>>
>>> I don't know how to reconcile 16.4 with -Wswitch. One could argue that
>>> -Wswitch could be a good way to address 16.4, but then we introduce a
>>> compiler specific requirement. Typically gcc is not the compiler of
>>> choice for these environments, unfortunately forcing gcc is not an
>>> option.
>>
>> Well, you could build with GCC and then build with your custom
>> compiler...
> 
> This suggestion is problematic: as an individual interested in MISRA-C
> compliance, I only have the MISRA-C rules in my hands. I don't know how
> to deal with suggestions like this one, that don't comply to the Rules,
> but it tries to address the same issue in a different manner.

Are you suggesting we will have to abide to all the rules even if they 
doesn't make things worst? I was under the impression we don't necessary 
need to follow a rule if we have justification for it.

> 
> I cannot rule out that it wouldn't work, but also I cannot be sure that
> it would work. In short, I have no way to make progress or to find out
> how to move forward. I guess as a contributor I would be forced to go
> back to the MISRAC compliance experts and ask for their opinion. (One
> non-technical issue is who is going to pay them for spending their time
> on this.) But what if they say it is not acceptable for compliance?

I appreciate people might want to use Xen with MISRA C. But I am not 
convinced we should bend to some rules in Xen Project for the sake of 
making MISRA happy. People could carry such patch themselves if they 
care about it.

> 
> This is a great topic to discuss in March and decide what to do in these
> situations.
> 
> 
>> But, GCC is pretty much the only choice for Xen on Arm today
>> as we don't build with clang and I pretty doubt we can build with compcert.
> 
> Obviously, this has to change if we want to make progress on safety
> certifications.

I am curious to know what is plan for this. I mean that if no-one is 
planning to make Xen build with other compilers. Then what would be the 
benefits of this?

Cheers,

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