* [XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1 @ 2024-03-20 8:50 Simone Ballarin 2024-03-20 8:50 ` [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin 2024-03-20 8:51 ` [XEN PATCH 2/2] xen: address violations of MISRA C " Simone Ballarin 0 siblings, 2 replies; 11+ messages in thread From: Simone Ballarin @ 2024-03-20 8:50 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, Doug Goldstein, Stefano Stabellini, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Wei Liu, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Roger Pau Monné MISRA C Rule 20.7 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. Functions hypercall_create_continuation and hypercall_xlat_continuation are special hypercalls made to break long running hypercalls into multiple calls. They take a variable number of arguments depending on the original hypercall they are trying to continue. Add SAF deviations for the aforementioned functions. Add deviation for functions related to console output (printk and similar). Simone Ballarin (2): automation/eclair: add deviation for MISRA C:2012 Rule 17.1 xen: address violations of MISRA C Rule 17.1 .../eclair_analysis/ECLAIR/deviations.ecl | 26 +++++++++++++++++++ docs/misra/deviations.rst | 5 ++++ docs/misra/safe.json | 8 ++++++ xen/arch/arm/domain.c | 1 + xen/arch/x86/hypercall.c | 2 ++ 5 files changed, 42 insertions(+) -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-20 8:50 [XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin @ 2024-03-20 8:50 ` Simone Ballarin 2024-03-20 9:08 ` Jan Beulich 2024-03-20 8:51 ` [XEN PATCH 2/2] xen: address violations of MISRA C " Simone Ballarin 1 sibling, 1 reply; 11+ messages in thread From: Simone Ballarin @ 2024-03-20 8:50 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, Doug Goldstein, Stefano Stabellini, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Wei Liu 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))))"} +-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))))"} +-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))))"} +-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 + # # Series 18. # diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index ce855ddae6..fa649a2024 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -328,6 +328,11 @@ Deviations related to MISRA C:2012 Rules: improve readability. - Tagged as `deliberate` for ECLAIR. + * - R17.1 + - Functions related to console output are allowed to use the variadic + features provided by `stdarg.h`. + - Tagged as `deliberate` for ECLAIR. + * - R20.7 - Code violating Rule 20.7 is safe when macro parameters are used: (1) as function arguments; -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-20 8:50 ` [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin @ 2024-03-20 9:08 ` Jan Beulich 2024-03-21 1:50 ` Stefano Stabellini 0 siblings, 1 reply; 11+ messages in thread From: Jan Beulich @ 2024-03-20 9:08 UTC (permalink / raw) To: Simone Ballarin Cc: consulting, Doug Goldstein, Stefano Stabellini, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel 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. > +-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. > +-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? Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-20 9:08 ` Jan Beulich @ 2024-03-21 1:50 ` Stefano Stabellini 2024-03-21 8:09 ` Jan Beulich 0 siblings, 1 reply; 11+ messages in thread From: Stefano Stabellini @ 2024-03-21 1:50 UTC (permalink / raw) To: Jan Beulich Cc: Simone Ballarin, consulting, Doug Goldstein, Stefano Stabellini, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-21 1:50 ` Stefano Stabellini @ 2024-03-21 8:09 ` Jan Beulich 2024-03-21 18:00 ` Stefano Stabellini 0 siblings, 1 reply; 11+ messages in thread From: Jan Beulich @ 2024-03-21 8:09 UTC (permalink / raw) To: Stefano Stabellini Cc: Simone Ballarin, consulting, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 21.03.2024 02:50, Stefano Stabellini wrote: > 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..." How about simply saying "printf()-like functions"? In what you suggest, sprintf() and friends would still not be covered. Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-21 8:09 ` Jan Beulich @ 2024-03-21 18:00 ` Stefano Stabellini 2024-03-22 8:08 ` Simone Ballarin 0 siblings, 1 reply; 11+ messages in thread From: Stefano Stabellini @ 2024-03-21 18:00 UTC (permalink / raw) To: Jan Beulich Cc: Stefano Stabellini, Simone Ballarin, consulting, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On Thu, 21 Mar 2024, Jan Beulich wrote: > On 21.03.2024 02:50, Stefano Stabellini wrote: > > 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..." > > How about simply saying "printf()-like functions"? In what you suggest, > sprintf() and friends would still not be covered. I like it ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-21 18:00 ` Stefano Stabellini @ 2024-03-22 8:08 ` Simone Ballarin 0 siblings, 0 replies; 11+ messages in thread From: Simone Ballarin @ 2024-03-22 8:08 UTC (permalink / raw) To: Stefano Stabellini, Jan Beulich Cc: consulting, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 21/03/24 19:00, Stefano Stabellini wrote: > On Thu, 21 Mar 2024, Jan Beulich wrote: >> On 21.03.2024 02:50, Stefano Stabellini wrote: >>> 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..." >> >> How about simply saying "printf()-like functions"? In what you suggest, >> sprintf() and friends would still not be covered. > > I like it > Ok, great. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com) ^ permalink raw reply [flat|nested] 11+ messages in thread
* [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1 2024-03-20 8:50 [XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin 2024-03-20 8:50 ` [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin @ 2024-03-20 8:51 ` Simone Ballarin 2024-03-20 9:11 ` Jan Beulich 1 sibling, 1 reply; 11+ messages in thread From: Simone Ballarin @ 2024-03-20 8:51 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Roger Pau Monné MISRA C Rule 20.7 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. Functions hypercall_create_continuation and hypercall_xlat_continuation are special hypercalls made to break long running hypercalls into multiple calls. They take a variable number of arguments depending on the original hypercall they are trying to continue. Add SAF deviations for the aforementioned functions. Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> --- docs/misra/safe.json | 8 ++++++++ xen/arch/arm/domain.c | 1 + xen/arch/x86/hypercall.c | 2 ++ 3 files changed, 11 insertions(+) diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 952324f85c..65c90c7618 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -28,6 +28,14 @@ }, { "id": "SAF-3-safe", + "analyser": { + "eclair": "MC3R1.R17.1" + }, + "name": "Rule 17.1: special hypercall made to break long running hypercalls into multiple calls.", + "text": "They need to take a variable number of arguments depending on the original hypercall they are trying to continue." + }, + { + "id": "SAF-4-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 5e7a7f3e7e..f5706bd5b8 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -382,6 +382,7 @@ unsigned long hypercall_create_continuation( const char *p = format; unsigned long arg, rc; unsigned int i; + /* SAF-3-safe allowed variadic function */ va_list args; current->hcall_preempted = true; diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c index 01cd73040d..18d8c75522 100644 --- a/xen/arch/x86/hypercall.c +++ b/xen/arch/x86/hypercall.c @@ -31,6 +31,7 @@ unsigned long hypercall_create_continuation( const char *p = format; unsigned long arg; unsigned int i; + /* SAF-3-safe allowed variadic function */ va_list args; curr->hcall_preempted = true; @@ -115,6 +116,7 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int nr, struct cpu_user_regs *regs; unsigned int i, cval = 0; unsigned long nval = 0; + /* SAF-3-safe allowed variadic function */ va_list args; ASSERT(nr <= ARRAY_SIZE(mcs->call.args)); -- 2.34.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1 2024-03-20 8:51 ` [XEN PATCH 2/2] xen: address violations of MISRA C " Simone Ballarin @ 2024-03-20 9:11 ` Jan Beulich 2024-03-21 1:47 ` Stefano Stabellini 0 siblings, 1 reply; 11+ messages in thread From: Jan Beulich @ 2024-03-20 9:11 UTC (permalink / raw) To: Simone Ballarin Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Roger Pau Monné, xen-devel On 20.03.2024 09:51, Simone Ballarin wrote: > MISRA C Rule 20.7 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. > > Functions hypercall_create_continuation and hypercall_xlat_continuation > are special hypercalls made to break long running hypercalls into multiple > calls. Here and below: These aren't "special hypercalls". They're internal helper functions. > They take a variable number of arguments depending on the original > hypercall they are trying to continue. Am I misremembering or did Andrew outline a plan to eliminate the variadic- ness from these? From certifiability perspective avoiding the need for a deviation would likely be preferable? Jan ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1 2024-03-20 9:11 ` Jan Beulich @ 2024-03-21 1:47 ` Stefano Stabellini 2024-03-22 8:30 ` Simone Ballarin 0 siblings, 1 reply; 11+ messages in thread From: Stefano Stabellini @ 2024-03-21 1:47 UTC (permalink / raw) To: Jan Beulich Cc: Simone Ballarin, consulting, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, Wei Liu, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Roger Pau Monné, xen-devel On Wed, 20 Mar 2024, Jan Beulich wrote: > On 20.03.2024 09:51, Simone Ballarin wrote: > > MISRA C Rule 20.7 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. > > > > Functions hypercall_create_continuation and hypercall_xlat_continuation > > are special hypercalls made to break long running hypercalls into multiple > > calls. > > Here and below: These aren't "special hypercalls". They're internal helper > functions. +1 > > They take a variable number of arguments depending on the original > > hypercall they are trying to continue. > > Am I misremembering or did Andrew outline a plan to eliminate the variadic- > ness from these? From certifiability perspective avoiding the need for a > deviation would likely be preferable? For sure, it would be preferable. In the meantime we can have the SAF comment? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1 2024-03-21 1:47 ` Stefano Stabellini @ 2024-03-22 8:30 ` Simone Ballarin 0 siblings, 0 replies; 11+ messages in thread From: Simone Ballarin @ 2024-03-22 8:30 UTC (permalink / raw) To: Stefano Stabellini, Jan Beulich Cc: consulting, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, Bertrand Marquis, Michal Orzel, Volodymyr Babchuk, Roger Pau Monné, xen-devel On 21/03/24 02:47, Stefano Stabellini wrote: > On Wed, 20 Mar 2024, Jan Beulich wrote: >> On 20.03.2024 09:51, Simone Ballarin wrote: >>> MISRA C Rule 20.7 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. >>> >>> Functions hypercall_create_continuation and hypercall_xlat_continuation >>> are special hypercalls made to break long running hypercalls into multiple >>> calls. >> >> Here and below: These aren't "special hypercalls". They're internal helper >> functions. > > +1 > > >>> They take a variable number of arguments depending on the original >>> hypercall they are trying to continue. >> >> Am I misremembering or did Andrew outline a plan to eliminate the variadic- >> ness from these? From certifiability perspective avoiding the need for a >> deviation would likely be preferable? > > For sure, it would be preferable. In the meantime we can have the SAF > comment? I agree in using the SAF comments as a temporary measure. I'll propose a new patch with the fix requested by Jan. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com) ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-03-22 8:30 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-20 8:50 [XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin 2024-03-20 8:50 ` [XEN PATCH 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin 2024-03-20 9:08 ` Jan Beulich 2024-03-21 1:50 ` Stefano Stabellini 2024-03-21 8:09 ` Jan Beulich 2024-03-21 18:00 ` Stefano Stabellini 2024-03-22 8:08 ` Simone Ballarin 2024-03-20 8:51 ` [XEN PATCH 2/2] xen: address violations of MISRA C " Simone Ballarin 2024-03-20 9:11 ` Jan Beulich 2024-03-21 1:47 ` Stefano Stabellini 2024-03-22 8:30 ` Simone Ballarin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.