Re: [PATCH 7/7] xen/device_tree: Fix MISRA C 2012 Rule 20.7 violations

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.08.2022 10:02, Xenia Ragiadakou wrote:
> On 8/22/22 14:48, Jan Beulich wrote:
>> On 22.08.2022 12:43, Xenia Ragiadakou wrote:
>>> On 8/22/22 12:59, Jan Beulich wrote:
>>>> On 19.08.2022 21:43, Xenia Ragiadakou wrote:
>>>>> In macros dt_for_each_property_node(), dt_for_each_device_node() and
>>>>> dt_for_each_child_node(), add parentheses around the macro parameters that
>>>>> have the arrow operator applied, to prevent against unintended expansions.
>>>>
>>>> Why is this relevant only when -> is used? For comparisons and the rhs of
>>>> assignments it's as relevant, ad even for the lhs of assignments I doubt
>>>> it can be generally omitted.
>>>
>>> Yes, I agree with you but some older patches that I sent that were
>>> adding parentheses around the lhs of the assignments were not accepted
>>> and I thought that the rhs of the assignments as well these comparisons
>>> fall to the same category.
>>>
>>> Personally, I would expect to see parentheses, also, around the macro
>>> parameters that are used as the lhs or the rhs of assignments, the
>>> operands of comparison or the arguments of a function.
>>> Not only because they can prevent against unintentional bugs but because
>>> the parentheses help me to identify more easily the macro parameters
>>> when reading a macro definition. I totally understand that for other
>>> people parentheses may reduce readability.
>>
>> Afair Julien's comments were very specific to the lhs of assignments.
>> So at the very least everything else ought to be parenthesized imo.
>>
> 
> So, IIUC, the only deviations from the MISRA C 2012 Rule 20.7 will be 
> for macro parameters used as the lhs of assignments and function arguments?

Afaic I don't consider that discussion settled.

> This feels a bit of a hack to parenthesize the macro parameters that are 
> used as the rhs of an assignment but not those used as the lhs.

lhs and rhs of assignments are quite different, and hence making such a
distinction wouldn't look to be a "hack" to me. In fact I've always
considered this part of the language somewhat strange: To me
parenthesizing e.g. an identifier already makes it (visually) an
rvalue. Leaving aside odd (and easy to spot as odd) uses at the call
sizes, I don't think I can come up with a case where parentheses are
really needed. Anything needing parenthesizing actually yields an
rvalue afaics, thus causing a diagnostic anyway.

>  From previous discussions on the topic, I understood that the 
> parentheses are considered needed only when they eliminate operator 
> precedence problems, while for the wrong parameter format bugs we can 
> rely on the reviewers.
> 
> I think we need to decide if the rule will be applied as is and if not 
> which will be the deviations along with their justification and add it 
> to the document.

Yes. But this shouldn't hinder adjustments for all other, non-
controversial cases.

Jan