Re: [XEN PATCH] xen/arm: p2m: refactor 'p2m_get_entry'

[Julien Grall <julien@xxxxxxx>]

On 24/07/2023 14:15, Nicola Vetrini wrote:
> On 21/07/23 12:01, Julien Grall wrote:
> > Hi Nicola,
> >
> > I would add "to please ECLAIR" in the commit title.
> I would be against this. It's the MISRA assessor that needs
> to understand and agree on what has been done to claim MISRA compliance. 
> A static analysis tool is just a useful way to help reaching this aim.
I have already mentioned in [1], but I will repeat here in a shorter 
version. If Eclair wasn't complaining about it, then this is not a patch 
I would have accepted with the current justification.
> > On 21/07/2023 07:49, Nicola Vetrini wrote:
> > > This function is refactored to avoid using a
> > > local dummy variable that served as a fallback
> > > if the parameter 't' is NULL.
> > >
> > > Storing the address of that variable into 't' caused
> > > static analysis tools not to be able to recognize the
> > Can you mention which static analysis tools is not happy and the 
> > version? This could help us in the future if we decided to revert the 
> > patch.
> It is not a matter of tool happiness, but of MISRA compliance.
From my understanding, the code you are modifying is compliant with 
MISRA. So you are only doing this patch to:
  1) Help Eclair to figure out there is no issue
  2) Avoid adding an explanation in the documentation why this is safe

> There are several tools (even emblazoned ones) that have lots
> of false negatives, also for mandatory guidelines such as Rule 9.1:
> the fact that they do not issue messages for possible violations
> they cannot exclude is happiness for nobody (especially for those
> who have to ensure there are indeed no violations). Two things
> that have to be kept well in mind are:
My point is that someone in the future may decide to use a tool that 
will complain about your change here. A possible course of action would 
be to revert the patch but this would not work for Eclair.
By explicitly mentioning Eclair in the commit message/title, you are 
making a statement that a simple revert would not work. This will save a 
lot of back and forth, including arguments on the ML.
Cheers,

--
Julien Grall