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

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Xenia,

> On 1 Sep 2022, at 10:27, Xenia Ragiadakou <burzalodowa@xxxxxxxxx> wrote:
> 
> 
> On 9/1/22 01:35, Stefano Stabellini wrote:
>> Patches 1, 4, and 6 are already committed. I plan to commit patches 2, 3
>> and 5 in the next couple of days.
>> Patch 7 needs further discussions and it is best addressed during the
>> next MISRA C sync-up.
> 
> I would like to share here, before the next MISRA C sync, my understandings 
> that will hopefully resolve a wrong impression of mine, that I may have 
> spread around, regarding this rule.
> There was a misunderstanding regarding the rule 20.7 from my part and I think 
> that Jan is absolutely right that parenthesizing macro parameters used as 
> function arguments is not required by the rule.
> 
> The rule 20.7 states "Expressions resulting from the expansion of macro 
> parameters shall be enclosed in parentheses" and in the rationale of the rule 
> states "If a macro parameter is not being used as an expression then the 
> parentheses are not necessary because no operators are involved.".
> 
> Initially, based on the title, my understanding was that it requires for the 
> expression resulting from the expansion of the macro to be enclosed in 
> parentheses. Then, based on the rule explanation and the examples given,  my 
> understanding was that it requires the macro parameters that are used as 
> expressions to be enclosed in parentheses.
> But, after re-thinking about it, the most probable and what makes more sense, 
> is that it require parentheses around the macro parameters that are part of 
> an expression and not around those that are used as expressions.
> 
> Therefore, macro parameters being used as function arguments are not required 
> to be enclosed in parentheses, because the function arguments are part of an 
> expression list, not of an expression (comma is evaluated as separator, not 
> as operator).
> While, macro parameters used as rhs and lhs expressions of the assignment 
> operator are required to be enclosed in parentheses because they are part of 
> an assignment expression.
> 
> I verified that the violation reported by cppcheck is not due to missing 
> parentheses around the function argument (though still I have not understood 
> the origin of the warning). Also, Eclair does not report it.
> 
> Hence, it was a misunderstanding of mine and there is no inconsistency, with 
> respect to this rule, in adding parentheses around macro parameters used as 
> rhs of assignments. The rule does not require adding parentheses around macro 
> parameters used as function arguments and neither cppcheck nor Eclair report 
> violation for missing parentheses around macro parameters used as function 
> arguments.


Thanks a lot for the detailed explanation :-)

What you say does make sense and I agree with your analysis here, only protect 
when part of an expression and not use as a subsequent parameter (for a 
function or an other macro).

Regards
Bertrand

> 
>> On Fri, 19 Aug 2022, Xenia Ragiadakou wrote:
>>> Xenia Ragiadakou (7):
>>>   xen/arm: gic_v3_its: Fix MISRA C 2012 Rule 20.7 violations
>>>   xsm/flask: sidtab: Fix MISRA C 2012 Rule 20.7 violations
>>>   xen/elf: Fix MISRA C 2012 Rule 20.7 violations
>>>   xen/vgic: Fix MISRA C 2012 Rule 20.7 violation
>>>   xen/rbtree: Fix MISRA C 2012 Rule 20.7 violation
>>>   xen/arm: processor: Fix MISRA C 2012 Rule 20.7 violations
>>>   xen/device_tree: Fix MISRA C 2012 Rule 20.7 violations
>>> 
>>>  xen/arch/arm/include/asm/gic_v3_its.h | 10 +++++-----
>>>  xen/arch/arm/include/asm/new_vgic.h   |  2 +-
>>>  xen/arch/arm/include/asm/processor.h  |  4 ++--
>>>  xen/include/xen/device_tree.h         |  6 +++---
>>>  xen/include/xen/elfstructs.h          |  4 ++--
>>>  xen/lib/rbtree.c                      |  2 +-
>>>  xen/xsm/flask/ss/sidtab.c             |  8 ++++----
>>>  7 files changed, 18 insertions(+), 18 deletions(-)
>>> 
>>> -- 
>>> 2.34.1
>>> 
> 
> -- 
> Xenia