Re: [XEN PATCH 5/7] xen/arm: v{cp,sys}reg: address violations of MISRA C:2012 Rule 16.3

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 20/12/2023 6:24 pm, Stefano Stabellini wrote:
> On Wed, 20 Dec 2023, Federico Serafini wrote:
>> On 20/12/23 12:55, Jan Beulich wrote:
>>> On 20.12.2023 12:48, Julien Grall wrote:
>>>> On 20/12/2023 11:03, Federico Serafini wrote:
>>>>> --- a/xen/arch/arm/arm64/vsysreg.c
>>>>> +++ b/xen/arch/arm/arm64/vsysreg.c
>>>>> @@ -210,8 +210,8 @@ void do_sysreg(struct cpu_user_regs *regs,
>>>>>            /* RO at EL0. RAZ/WI at EL1 */
>>>>>            if ( regs_mode_is_user(regs) )
>>>>>                return handle_ro_raz(regs, regidx, hsr.sysreg.read, hsr,
>>>>> 0);
>>>>> -        else
>>>>> -            return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr,
>>>>> 1);
>>>>> +
>>>>> +        return handle_raz_wi(regs, regidx, hsr.sysreg.read, hsr, 1);
>>>> I don't 100% like this change (mostly because I find if/else clearer
>>>> here).
>>> While (it doesn't matter here) my view on this is different, I'm still
>>> puzzled why the tool would complain / why a change here is necessary.
>>> It is not _one_ return statement, but there's still (and obviously) no
>>> way of falling through.
>> The tool is configurable:
>> if you prefer deviate these cases instead of refactoring the code
>> I can update the configuration.
>  
> If you say "deviation", it implies that there is a violation. I think
> Jan's point was that both code versions shouldn't be any different.
>
> # option-1
> if (a)
>   return f1();
> else
>   return f2();
>
> # option-2
> if (a)
>   return f1();
> return f2();
>
> Both options are equally guaranteed to always return. It looks like a
> peculiar limitation to only recognize option-2 as something that returns
> 100% of the times. Also option-1 returns 100% of the times. What am I
> missing?

For completeness, it's worth saying that there is an option-3:

    return a ? f1() : f2();

which is very clearly only a single return, but I personally don't like
this as I often find it to be less clear than either other option.

All options have a guaranteed return, but there cases including this one
where option-1 is clearest way to write the logic.

~Andrew