Re: [XEN PATCH v2 1/6][RESEND] automation/eclair: address violations of MISRA C Rule 20.7

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 25 Jun 2024, Nicola Vetrini wrote:
> On 2024-06-21 02:18, Stefano Stabellini wrote:
> > On Mon, 16 Jun 2024, Nicola Vetrini wrote:
> > > MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> > > of macro parameters shall be enclosed in parentheses".
> > > 
> > > The helper macro bitmap_switch has parameters that cannot be parenthesized
> > > in order to comply with the rule, as that would break its functionality.
> > > Moreover, the risk of misuse due developer confusion is deemed not
> > > substantial enough to warrant a more involved refactor, thus the macro
> > > is deviated for this rule.
> > > 
> > > No functional change.
> > > 
> > > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > 
> > If possible, I would prefer we used a SAF in-code comment deviation. If
> > that doesn't work for any reason this patch is fine and I'd ack it.
> > 
> 
> Would that be an improvement for safety in your opinion?

Not for safety, as they both achieve the same result, but for
maintainability. The deviations under deviations.ecl are project-wide so
in my opinion should only be used for project-wide global deviations
from a rule. Specific deviations for functions should be done with SAF.
Another reason for this is that deviations under deviations.ecl need to
be manually ported to any other static analyzer one by one while SAF is
meant to support multiple automatically.

More importantly if we change deviations.ecl, deviations.rst should also
be changed the same way. I would say that this is required at a minimum
as deviations.ecl and deviations.rst should be in sync.

 
> > > ---
> > >  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > index 447c1e6661d1..c2698e7074aa 100644
> > > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > @@ -463,6 +463,14 @@ of this macro do not lead to developer confusion, and
> > > can thus be deviated."
> > >  -config=MC3R1.R20.7,reports+={safe,
> > > "any_area(any_loc(any_exp(macro(^count_args_$))))"}
> > >  -doc_end
> > > 
> > > +-doc_begin="The arguments of macro bitmap_switch macro can't be
> > > parenthesized as
> > > +the rule would require, without breaking the functionality of the macro.
> > > This is
> > > +a specialized local helper macro only used within the bitmap.h header, so
> > > it is
> > > +less likely to lead to developer confusion and it is deemed better to
> > > deviate it."
> > > +-file_tag+={xen_bitmap_h, "^xen/include/xen/bitmap\\.h$"}
> > > +-config=MC3R1.R20.7,reports+={safe,
> > > "any_area(any_loc(any_exp(macro(loc(file(xen_bitmap_h))&&^bitmap_switch$))))"}
> > > +-doc_end
> > > +
> > >  -doc_begin="Uses of variadic macros that have one of their arguments
> > > defined as
> > >  a macro and used within the body for both ordinary parameter expansion
> > > and as an
> > >  operand to the # or ## operators have a behavior that is well-understood
> > > and
> > > --
> > > 2.34.1
> > > 
> 
> -- 
> Nicola Vetrini, BSc
> Software Engineer, BUGSENG srl (https://bugseng.com)
>