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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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


> > 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?


 
> > If we cannot generically claim that rhs parentheses are never needed,
> > then I don't think we should make any exceptions. We should add them here
> > and everywhere else. It should be easy to write a macro or review a
> > patch with a macro from someone else, and making special exception makes
> > it more difficult for everyone.