Re: [PATCH v3 1/2] xen/device_tree: fix Eclair findings for MISRA C 2012 Rule 20.7

[Xenia Ragiadakou <burzalodowa@xxxxxxxxx>]

Hi Julien,

On 2/7/23 15:01, Julien Grall wrote:
> Hi,
>
> On 07/02/2023 12:46, Xenia Ragiadakou wrote:
> > On 2/7/23 14:25, Julien Grall wrote:
> > > On 07/02/2023 10:46, Xenia Ragiadakou wrote:
> > > > On 2/7/23 12:39, Julien Grall wrote:
> > > > > Hi,
> > > > >
> > > > > On 07/02/2023 10:23, Luca Fancellu wrote:
> > > > > > > On 3 Feb 2023, at 19:09, Xenia Ragiadakou <burzalodowa@xxxxxxxxx> 
> > > > > > > wrote:
> > > > > > I’m not really a supporter of empty commit message, but it’s up to 
> > > > > > the maintainer :)
> > > > > +1. In this case a brief summary of the rule would be handy for 
> > > > > those that are not well-versed with MISRA.
> > > > > This can be dealt on commit if you propose a new commit message.
> > > > I 'm refrained from stating the rule as is because it is strict and 
> > > > it is not applied as is.
> > > I am a bit confused with this statement. From misra/..., we are 
> > > supporting rule 20.7. So why aren't applying it strictly?
> > > And if it is not applied as-is, shouldn't we document the violation 
> > > (if any)?
> > I applied it strictly on v2, but there was no review.
> Ah! In general it is best to ping if there are no answers.
Me too I ve forgotten about it. A ticket in gitlab reminded me that it 
was pending.
> > Then Eclair was adjusted to have a less strict approach. Still there 
> > is a finding asking to add parentheses around dt in 
> > dt_for_each_device_node(dt, dn), i.e dn = (dt);, to which AFAIK you 
> > object.
> Are you referring to the discussion in [1]? If so, I wasn't totally 
> opposed to the suggestion so long we are consistent.
I am not able to find [1]. I 'm referring to the discussion in 
20221220085100.22848-6-luca.fancellu@xxxxxxx and 
20220728134943.1185621-1-burzalodowa@xxxxxxxxx
> > > > "Add parentheses around macro parameters when the precedence and 
> > > > associativity of the performed operators can lead to unintended 
> > > > order of evaluation."
> > > > Is this ok?
> > > I am OK with this. Is there any ID from Eclair that could be used to 
> > > track each error (and so we can confirm they have disappeared)?
> > I am not aware of any.
> Hmmm ok. It might be a nice feature to suggest in Eclair because anyone 
> can check whether an issue was resolved.
Currently, the violations in Eclair are reported per macro call (I guess 
because it is acceptable to have parentheses around the macro args) so 
it is difficult to track all of them.
> Here, I don't exactly know what to check in Eclair. So I have to rely on 
> you. Anyway, nothing that can be fixed for this commit.
> > The patch can be decoupled from misra and Eclair (I mean have a 
> > generic commit title) and just mention in the commit message that it 
> > fixes some Eclair findings for MISRA C rule 20.7.
> I have a slight preference for a more generic title. But the current one 
> also work for me.
It can be changed into "xen/device_tree: add parentheses around macro 
parameters"
> I will commit later on.
Do you want me to send a v4?

> Cheers,
>
> [1] b2f2d1e7-0c18-206f-5e9d-d0115e398840@xxxxxxx
> > > Cheers,
--
Xenia