Re: [XEN PATCH v2 3/3] eclair: add deviations of MISRA C Rule 5.5

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-07-12 03:13, Stefano Stabellini wrote:
> On Fri, 11 Jul 2025, Nicola Vetrini wrote:
> > On 2025-07-09 23:38, Dmytro Prokopchuk1 wrote:
> > > MISRA C Rule 5.5 states that: "Identifiers shall
> > > be distinct from macro names".
> > >
> > > Update ECLAIR configuration to deviate:
> > > - clashes in 'xen/include/xen/bitops.h';
> > > - clashes in 'xen/include/xen/irq.h';
> > > - clashes in 'xen/common/grant_table.c'.
> > >
> > > Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@xxxxxxxx>
> > > ---
> > >  automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++
> > >  docs/misra/deviations.rst                        | 8 ++++++++
> > >  2 files changed, 16 insertions(+)
> > >
> > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > index e8f513fbc5..a5d7b00094 100644
> > > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > > @@ -117,6 +117,14 @@ it defines would (in the common case) be already
> > > defined. Peer reviewed by the c
> > >  -config=MC3A2.R5.5,reports+={deliberate,
> > > 
> > "any_area(decl(kind(function))||any_loc(macro(name(memcpy||memset||memmove))))&&any_area(any_loc(file(^xen/common/libelf/libelf-private\\.h$)))"}
> > >  -doc_end
> > >
> > > +-doc_begin="Clashes between function names and macros are deliberate for
> > > bitops functions, pirq_cleanup_check, update_gnttab_par and
> > > parse_gnttab_limit functions
> > > +and needed to have a function-like macro that acts as a wrapper for the
> > > function to be called. Before calling the function,
> > > +the macro adds additional checks or adjusts the number of parameters
> > > depending on the configuration."
> > > +-config=MC3A2.R5.5,reports+={deliberate,
> > > "any_area(all_loc(file(^xen/include/xen/bitops\\.h$)))"}
> >
> > Bitops violations are not inside "xen/include/xen/bitops.h", but 
> > rather
> > "xen/arch/x86/include/asm/bitops.h"
> >
> > > +-config=MC3A2.R5.5,reports+={deliberate,
> > > 
> > "any_area(all_loc(file(^xen/include/xen/irq\\.h$))&&context(name(pirq_cleanup_check)&&kind(function)))"}
> >
> > I would rather do (untested)
> >
> > -config=MC3A2.R5.5,reports+={deliberate,
> > "all_area(decl(name(pirq_cleanup_check))||macro(name(pirq_cleanup_check)))"}
> >
> > > +-config=MC3A2.R5.5,reports+={deliberate,
> > > 
> > "any_area(all_loc(file(^xen/common/grant_table\\.c$))&&context(name(update_gnttab_par||parse_gnttab_limit)&&kind(function)))"}
> > > +-doc_end
> > > +
> >
> > same as above
> Thanks Nicola! The following deviations are enough and sufficient to
> zero violations on both ARM and x86:
>
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^xen/include/xen/bitops\\.h$)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^xen/arch/x86/include/asm/bitops\\.h$)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^.*/compat\\.c$)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^.*/compat/.*$)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^xen/arch/x86/x86_emulate/.*$)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "any_area(all_loc(file(^xen/arch/x86/include/asm/genapic\\.h$)))"}
Thinking a bit more about it, this deviation is perhaps a bit too wide, 
though in this case it's probably fine. Probably what is actually wanted 
is "all_area(all_loc(...))" . This ensures that the decl area and the 
macro area of the report are in the same file, which is almost always 
the case for deliberate shadowing. If that turns out to be too strict, 
then we may do "all_area(any_loc(...))", ensuring that at least there is 
a loc for the macro and decl in that file.
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(parse_gnttab_limit))||macro(name(parse_gnttab_limit)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(update_gnttab_par))||macro(name(update_gnttab_par)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(pirq_cleanup_check))||macro(name(pirq_cleanup_check)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(virt_to_maddr))||macro(name(virt_to_maddr)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(set_px_pminfo))||macro(name(set_px_pminfo)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(set_cx_pminfo))||macro(name(set_cx_pminfo)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(cpu_has_amd_erratum))||macro(name(cpu_has_amd_erratum)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(copy_to_guest_ll))||macro(name(copy_to_guest_ll)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(copy_from_guest_ll))||macro(name(copy_from_guest_ll)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(edd_put_string))||macro(name(edd_put_string)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(cpu_has_amd_erratum))||macro(name(cpu_has_amd_erratum)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(page_list_entry))||macro(name(page_list_entry)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(do_physdev_op))||macro(name(do_physdev_op)))"}
> -config=MC3A2.R5.5,reports+={deliberate, 
> "all_area(decl(name(do_platform_op))||macro(name(do_platform_op)))"}
> Jan, are you OK with it?
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253