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

[Julien Grall <julien@xxxxxxx>]

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.

> 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.
> > > "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.
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.
I will commit later on.

Cheers,

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