On 20.02.2024 09:16, Federico Serafini wrote:
> On 19/02/24 16:06, Jan Beulich wrote:
>> On 19.02.2024 15:59, Federico Serafini wrote:
>>> On 19/02/24 14:43, Jan Beulich wrote:
>>>> On 19.02.2024 14:24, Federico Serafini wrote:
>>>>> Update ECLAIR configuration to consider safe switch clauses ending
>>>>> with __{get,put}_user_bad().
>>>>>
>>>>> Update docs/misra/deviations.rst accordingly.
>>>>>
>>>>> Signed-off-by: Federico Serafini <[email protected]>
>>>>
>>>> As mentioned I'm not happy with this, not the least because of it being
>>>> unclear why these two would be deviated, when there's no sign of a
>>>> similar deviation for, say, __bad_atomic_size(). Imo this approach
>>>> doesn't scale, and that's already leaving aside that the purpose of
>>>> identically named (pseudo-)helpers could differ between architectures,
>>>> thus putting under question ...
>>>>
>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> @@ -368,6 +368,10 @@ safe."
>>>>> -config=MC3R1.R16.3,reports+={safe,
>>>>> "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>>>> -doc_end
>>>>>
>>>>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\"
>>>>> and \"__put_user_bad()\" are safe: they denote an unreachable program
>>>>> point."
>>>>> +-config=MC3R1.R16.3,reports+={safe,
>>>>> "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>>>>> +-doc_end
>>>>
>>>> ... the global scope of such a deviation. While it may not be a good idea,
>>>> even within an arch such (pseudo-)helpers could be used for multiple
>>>> distinct purposes.
>>>
>>> Would you agree with adding the missing break statement after
>>> the uses of __{put,get}_user_bad() (as it is already happening for
>>> uses of __bad_atomic_size())?
>>
>> I probably wouldn't mind that (despite being a little pointless).
>> Perhaps declaring them as noreturn would also help?
>
> Yes, it will help.
> Is there any reason to have long as __get_user_bad()'s return value?
> It would be nicer to declare it as a void function and then add the
> noreturn attribute.
That's a historical leftover, which can be changed. Xen originally
derived quite a bit of code from Linux. If you go look at Linux 2.6.16,
you'll find why it was declared that way.
Jan
