Re: [XEN PATCH 10/10] xen/keyhandler: address violations of MISRA C Rule 20.7

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.03.2024 02:39, Stefano Stabellini wrote:
> On Tue, 5 Mar 2024, Jan Beulich wrote:
>> On 05.03.2024 03:03, Stefano Stabellini wrote:
>>> On Mon, 4 Mar 2024, Jan Beulich wrote:
>>>> On 02.03.2024 02:37, Stefano Stabellini wrote:
>>>>> On Fri, 1 Mar 2024, Jan Beulich wrote:
>>>>>> On 29.02.2024 23:57, Stefano Stabellini wrote:
>>>>>>> On Thu, 29 Feb 2024, Nicola Vetrini wrote:
>>>>>>>> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
>>>>>>>> of macro parameters shall be enclosed in parentheses". Therefore, some
>>>>>>>> macro definitions should gain additional parentheses to ensure that all
>>>>>>>> current and future users will be safe with respect to expansions that
>>>>>>>> can possibly alter the semantics of the passed-in macro parameter.
>>>>>>>>
>>>>>>>> No functional change.
>>>>>>>>
>>>>>>>> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>>>>>>>
>>>>>>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>>>>>
>>>>>> You did see the discussion on earlier patches, though? I don't think
>>>>>> any of the parentheses here are needed or wanted.
>>>>>
>>>>> We need to align on this. Currently if we go by what's written in
>>>>> docs/misra/deviations.rst, then rhs should have parentheses.
>>>>
>>>> Quoting the actual patch again:
>>>
>>> [...]
>>>
>>>> What rhs are you talking about in light of this change? The only rhs I
>>>> can spot here is already parenthesized.
>>>
>>> Yes you are right. I replied here as an overall comment about our
>>> approach to 20.7, although this patch is not a good example. My reply
>>> was meant in the context of https://marc.info/?l=xen-devel&m=170928051025701
>>
>> I'm still confused: The rhs is being parenthsized there. It's the _lhs_
>> which isn't and ...
>>
>>>>> Can we safely claim that rhs parentheses are never needed? If so, then
>>>>> great, let's add it to deviations.rst and skip them here and other
>>>>> places in this patch series (e.g. patch #8). When I say "never" I am
>>>>> taking for granted that the caller is not doing something completely
>>>>> unacceptably broken such as: 
>>>>>
>>>>>      WRITE_SYSREG64(var +, TTBR0_EL1)
>>>>
>>>> I'm afraid I can't associate this with the patch here either. Instead in
>>>> the context here a (respective) construct as you mention above would simply
>>>> fail to build.
>>>
>>> Fair enough it will break the build. I was trying to clarify that when I
>>> wrote "the rhs parentheses are never needed" I meant "never" within
>>> reason. One can always find ways to break the system and I tried to make
>>> an example of something that for sure would break rhs or lhs without
>>> parentheses.
>>>
>>> I meant to say, if we don't account for exceptionally broken cases, can
>>> we safety say we don't need parentheses for rhs?
>>
>> ... doesn't need to, unless - as you say - one contrives examples. Yet to
>> clarify here as well: I assume you mean "we don't need parentheses for lhs".
> 
> Yes. Far clarity, we are all aligned that lhs does not need parentheses
> and in fact it is even already deviated in docs/misra/deviations.rst.
> 
> Now back to this specific patch.
> 
> The problem is that the comma ',' doesn't need parenthesis for the
> parameters. However, the way we wrote the deviation in
> docs/misra/deviations.rst doesn't cover the case this patch is dealing
> with. docs/misra/deviations.rst only speaks of functions and macros
> arguments.
> 
> Should we rephrase docs/misra/deviations.rst in terms of ',' instead ?

That's what I've requested elsewhere, yes.

> Can ECLAR deal with it?

I seem to vaguely recall Nicola saying it can, but if not then it surely
can be made to do so.

Jan