Re: MISRA C Rule 20.7 disambiguation

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 9 Dec 2022, Jan Beulich wrote:
> On 09.12.2022 01:45, Stefano Stabellini wrote:
> > This patch is to start a discussion in regard to rule 20.7 and its
> > interpretation. During the last MISRA C call we discussed that "our"> 
> > interpretation of the rule means that the following two cases don't need
> > extra parenthesis:
> > 
> > #define M(a, b) func(a, b)
> > #define M(a, b) (a) = b
> 
> I'm puzzled by the latter.

Sorry that was my mistake, I misread one of the examples.


> Iirc there was discussion on whether the LHS of an assignment needs
> parentheses, but I don't think there was any question about the RHS
> wanting them, irrespective of the facts that only comma expressions
> have lower precedence than assignment ones and that evaluation goes
> right to left anyway.
> 
> One aspect speaking for parentheses even on the LHS is that an expression
> (rather than an lvalue) passed as macro argument then uniformly becomes
> invalid, i.e. not just
> 
>       M(x + y, z)
> 
> would be rejected by the compiler, but also
> 
>       M(x = y, z)

So actually it should be:

#define M(a, b) (a) = (b)


> > Let's take another example:
> > 
> > #define xzalloc_flex_struct(type, field, nr) \
> >     ((type *)_xzalloc(offsetof(type, field[nr]), __alignof__(type)))
> > 
> > "type" is the same as last time. There are 2 other interesting macro
> > parameters here: nr and field.
> > 
> > nr could result in an expression, but I don't think it needs
> > parenthesis because it is between []? However, we know we have a clear
> > exception for the "," operator. We don't have a clear exception for the
> > [] operator. Do we need (nr)?
> 
> The question of whether parentheses are needed clearly need to be based
> on whether without parentheses anything that looks sensible on the surface
> can be mistaken for other than what was meant. I think [] simply are
> another form of parenthesization, even if these are commonly called square
> bracket (not parentheses). For this to be mistaken, a macro argument would
> need to be passed which first has a ] and then a [. This clearly doesn't
> look sensible even when just very briefly looking at it. Plus the same
> issue would exist with parentheses: You could also undermine the proper
> use of parentheses in the macro by passing a macro argument which first
> has ) and then (. IOW - adding parentheses here adds no value, and hence
> is merely clutter.
> 
> > field could result in an expression, so I think it needs parenthesis.
> 
> No, field (and intentionally named that way) is a struct member indicator.
> Neither p->(field) nor s.(field) are syntactically valid. There simply
> cannot be parentheses here, so the same conclusion as near the top applies.
>
> > Just to be clear, I'll list all the possible options below.
> > 
> > a) no changes needed, xzalloc_flex_struct is good as is
> 
> This is it, and not surprisingly: This construct was introduced not that
> long ago, when we already paid close attention to parenthesization needs.


Let's see if we can configure a MISRA C checking tool with the behavior
we would like, i.e. xzalloc_flex_struct not a violation. (I am using
xzalloc_flex_struct as an example.)

cppcheck marks xzalloc_flex_struct as violation but it is not surprising
as we know cppcheck is a bit rudimental. It might be better to disable
scanning for 20.7 in cppcheck.

Eclair marks it as a violation too. Eclair thinks "nr" needs
parenthesis. Roberto, we have already discussed how the comma operator
"," being the lower precedence doesn't require extra parenthesis.
Roberto, what's your take on the [] square brakets?