Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1

From: Stefano Stabellini <sstabellini@kernel.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Simone Ballarin <simone.ballarin@bugseng.com>,
	consulting@bugseng.com,  Doug Goldstein <cardoe@cardoe.com>,
	 Stefano Stabellini <sstabellini@kernel.org>,
	 Andrew Cooper <andrew.cooper3@citrix.com>,
	 George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien@xen.org>,  Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1
Date: Wed, 20 Mar 2024 18:50:48 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.22.394.2403201848340.1569010@ubuntu-linux-20-04-desktop> (raw)
In-Reply-To: <9341a711-74c3-463f-8cc2-a54e19b4b2f4@suse.com>

On Wed, 20 Mar 2024, Jan Beulich wrote:
> On 20.03.2024 09:50, Simone Ballarin wrote:
> > MISRA C:2012 Rule 17.1 states:
> > The features of `<stdarg.h>' shall not be used
> > 
> > The Xen community wants to avoid using variadic functions except for
> > specific circumstances where it feels appropriate by strict code review.
> > 
> > Add deviation for functions related to console output (printk and similar).
> > 
> > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
> > ---
> >  .../eclair_analysis/ECLAIR/deviations.ecl     | 26 +++++++++++++++++++
> >  docs/misra/deviations.rst                     |  5 ++++
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > index 9ac3ee4dfd..7c3559a3a0 100644
> > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> > @@ -382,6 +382,32 @@ explicit comment indicating the fallthrough intention is present."
> >  -config=MC3R1.R16.6,switch_clauses+={deliberate, "default(0)"}
> >  -doc_end
> >  
> > +#
> > +# Series 17.
> > +#
> > +
> > +-doc_begin="Functions related to console output are allowed to use the variadic features provided by stdarg.h."
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(debugtrace_printk)&&kind(function))))"}
> 
> This isn't concole output related.

Should we say "related to console output or tracing" ?

> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(dt_dprintk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(printk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(guest_printk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(panic)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(dprintk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(gdprintk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(snprintf)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(scnprintf)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(xasprintf)&&kind(function))))"}
> 
> These three and ...
> 
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(mm_printk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(dbgp_printk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(elf_call_log_callback)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(vsnprintf)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(vscnprintf)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(xvasprintf)&&kind(function))))"}
> 
> ... these three aren't either.

Maybe it is better to write it as:

"Functions related printk, logging and tracing are allowed..."

> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(vprintk_common)&&kind(function))))"}
> > +-config=MC3R1.R17.1,reports+={deliberate,"any_area(^.*va_list.*$&&context(ancestor_or_self(name(guest_printk)&&kind(function))))"}
> > +-config=MC3R1.R17.1,macros+={hide , "^va_(arg|start|copy|end)$"}
> > +-doc_end
> 
> Further, is there a reason xyzprintk() are all listed individually?
> Surely if we'd introduce a new flavor, we'd want that excluded too.
> 
> Finally, {,g}dprintk() in neither of their incarnation use any of the
> va_* items listed. Why do they need mentioning here?