Re: [XEN PATCH 2/2] xen/cpu: address MISRA C Rule 17.7

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.02.2024 01:26, Stefano Stabellini wrote:
> On Mon, 26 Feb 2024, Jan Beulich wrote:
>> On 23.02.2024 10:35, Nicola Vetrini wrote:
>>> Refactor cpu_notifier_call_chain into two functions:
>>> - the variant that is allowed to fail loses the nofail flag
>>> - the variant that shouldn't fail is encapsulated in a call
>>>   to the failing variant, with an additional check.
>>>
>>> This prevents uses of the function that are not supposed to
>>> fail from ignoring the return value, thus violating Rule 17.7:
>>> "The value returned by a function having non-void return type shall
>>> be used".
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>>
>> I'm afraid I disagree with this kind of bifurcation. No matter what
>> Misra thinks or says, it is normal for return values of functions to
>> not always be relevant to check.
> 
> Hi Jan, I disagree.
> 
> Regardless of MISRA, I really think return values need to be checked.
> Moreover, we decided as a group to honor MISRA Rule 17.7, which requires
> return values to be checked. This patch is a good step forward.

Yet splitting functions isn't the only way to deal with Misra's
requirements, I suppose. After all there are functions where the
return value is purely courtesy for perhaps just one of its callers.

Splitting simply doesn't scale very well, imo.

>> To deal with the Misra rule imo requires to first have an abstract
>> plan of how to handle such globally in the code base. Imo such a plan
>> can't be to introduce perhaps dozens of new wrapper functions like is
>> done here.
> 
> This patch is following the right pattern, one we already follow with
> the _locked suffix.

Right, and - just to mention it - one which I similarly dislike, albeit
to a lesser degree.

Jan