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
