* [XEN PATCH v2 0/2] xen: address violations of MISRA C Rule 17.1 @ 2024-03-22 8:46 Simone Ballarin 2024-03-22 8:46 ` [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin 2024-03-22 8:46 ` [XEN PATCH v2 2/2] xen: address violations of MISRA C " Simone Ballarin 0 siblings, 2 replies; 7+ messages in thread From: Simone Ballarin @ 2024-03-22 8:46 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, Doug Goldstein, 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 internal helpers 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 printf()-like functions. --- Changes in v2: - replace "related to console output" with "printf()-like functions"; - replace "special hypercalls" with "internal helpers". 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] 7+ messages in thread
* [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-22 8:46 [XEN PATCH v2 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin @ 2024-03-22 8:46 ` Simone Ballarin 2024-03-22 9:44 ` Jan Beulich 2024-03-22 8:46 ` [XEN PATCH v2 2/2] xen: address violations of MISRA C " Simone Ballarin 1 sibling, 1 reply; 7+ messages in thread From: Simone Ballarin @ 2024-03-22 8:46 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, Doug Goldstein, 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 printf()-like functions. Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> --- Changes in v2: - replace "console output related" with "printf()-like". --- .../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..334ee674c4 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="printf()-like functions 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..1e617f91f3 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 + - printf()-like functions 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] 7+ messages in thread
* Re: [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-22 8:46 ` [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin @ 2024-03-22 9:44 ` Jan Beulich 2024-03-22 13:44 ` Simone Ballarin 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2024-03-22 9:44 UTC (permalink / raw) To: Simone Ballarin Cc: consulting, sstabellini, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 22.03.2024 09:46, Simone Ballarin wrote: > --- 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="printf()-like functions 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 Repeating v1 comments which weren't addressed: Is there a reason xyzprintk() are all listed individually? Surely if we'd introduce a new flavor, we'd want that excluded too. {,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] 7+ messages in thread
* Re: [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-22 9:44 ` Jan Beulich @ 2024-03-22 13:44 ` Simone Ballarin 2024-03-22 14:12 ` Jan Beulich 0 siblings, 1 reply; 7+ messages in thread From: Simone Ballarin @ 2024-03-22 13:44 UTC (permalink / raw) To: Jan Beulich Cc: consulting, sstabellini, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 22/03/24 10:44, Jan Beulich wrote: > On 22.03.2024 09:46, Simone Ballarin wrote: >> --- 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="printf()-like functions 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 > > Repeating v1 comments which weren't addressed> > Is there a reason xyzprintk() are all listed individually? Surely if > we'd introduce a new flavor, we'd want that excluded too. > > {,g}dprintk() in neither of their incarnation use any of the va_* > items listed. Why do they need mentioning here? > > Jan Sorry. What do you mean with "xyzprintk()"? Maybe you mean a regex-based deviation using ".*printk\(.*\)"? That would unnecessarily deviate "{,g}dprintk()" as you said. Moreover, to cover also "vprintk_common", we would instead need something like "^.*printk.*\(.*\)$". I'm fine in using "^.*printk.*\(.*\)$", I just preferred the individual whitelisting to avoid unwanted exclusions. P.S: using regex I cannot match only the name, but I have to match the entire signature. -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-22 13:44 ` Simone Ballarin @ 2024-03-22 14:12 ` Jan Beulich 2024-03-22 14:24 ` Simone Ballarin 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2024-03-22 14:12 UTC (permalink / raw) To: Simone Ballarin Cc: consulting, sstabellini, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 22.03.2024 14:44, Simone Ballarin wrote: > On 22/03/24 10:44, Jan Beulich wrote: >> On 22.03.2024 09:46, Simone Ballarin wrote: >>> --- 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="printf()-like functions 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 >> >> Repeating v1 comments which weren't addressed> >> Is there a reason xyzprintk() are all listed individually? Surely if >> we'd introduce a new flavor, we'd want that excluded too. >>> {,g}dprintk() in neither of their incarnation use any of the va_* >> items listed. Why do they need mentioning here? > > What do you mean with "xyzprintk()"? Maybe you mean a regex-based > deviation using ".*printk\(.*\)"? Regex or globbing, whatever is available. > That would unnecessarily deviate "{,g}dprintk()" as you said. Depends on how the regex would be written. > Moreover, to cover also "vprintk_common", we would instead need something like > "^.*printk.*\(.*\)$". Or simply accept that such outliers need listing individually. That would imo still be better than a dozen entries. > I'm fine in using "^.*printk.*\(.*\)$", I just preferred the individual > whitelisting to avoid unwanted exclusions. > > P.S: > using regex I cannot match only the name, but I have to match > the entire signature. Interesting. Is that a tool limitation? Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 Rule 17.1 2024-03-22 14:12 ` Jan Beulich @ 2024-03-22 14:24 ` Simone Ballarin 0 siblings, 0 replies; 7+ messages in thread From: Simone Ballarin @ 2024-03-22 14:24 UTC (permalink / raw) To: Jan Beulich Cc: consulting, sstabellini, Doug Goldstein, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu, xen-devel On 22/03/24 15:12, Jan Beulich wrote: > On 22.03.2024 14:44, Simone Ballarin wrote: >> On 22/03/24 10:44, Jan Beulich wrote: >>> On 22.03.2024 09:46, Simone Ballarin wrote: >>>> --- 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="printf()-like functions 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 >>> >>> Repeating v1 comments which weren't addressed> >>> Is there a reason xyzprintk() are all listed individually? Surely if >>> we'd introduce a new flavor, we'd want that excluded too. >>>> {,g}dprintk() in neither of their incarnation use any of the va_* >>> items listed. Why do they need mentioning here? >> >> What do you mean with "xyzprintk()"? Maybe you mean a regex-based >> deviation using ".*printk\(.*\)"? > > Regex or globbing, whatever is available. > >> That would unnecessarily deviate "{,g}dprintk()" as you said. > > Depends on how the regex would be written. > >> Moreover, to cover also "vprintk_common", we would instead need something like >> "^.*printk.*\(.*\)$". > > Or simply accept that such outliers need listing individually. That > would imo still be better than a dozen entries. Ok, I will propose a patch using "^.*printk\(.*\)$" > >> I'm fine in using "^.*printk.*\(.*\)$", I just preferred the individual >> whitelisting to avoid unwanted exclusions. >> >> P.S: >> using regex I cannot match only the name, but I have to match >> the entire signature. > > Interesting. Is that a tool limitation? Why limitation? I wrote the P.S. just to motivate why I've added the "\(.*\)". "name(abc)&&kind(function)" will simply become "^abc\(.*\)$". Please note that I've removed "kind(function)" since the regex can only match functions. > > Jan > -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [XEN PATCH v2 2/2] xen: address violations of MISRA C Rule 17.1 2024-03-22 8:46 [XEN PATCH v2 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin 2024-03-22 8:46 ` [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin @ 2024-03-22 8:46 ` Simone Ballarin 1 sibling, 0 replies; 7+ messages in thread From: Simone Ballarin @ 2024-03-22 8:46 UTC (permalink / raw) To: xen-devel Cc: consulting, sstabellini, Simone Ballarin, 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 internal helper functions 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> --- Changes in v2: - replaced "special hypercalls" with "internal helper functions". --- 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..1145967213 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: internal helper functions 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] 7+ messages in thread
end of thread, other threads:[~2024-03-22 14:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-22 8:46 [XEN PATCH v2 0/2] xen: address violations of MISRA C Rule 17.1 Simone Ballarin 2024-03-22 8:46 ` [XEN PATCH v2 1/2] automation/eclair: add deviation for MISRA C:2012 " Simone Ballarin 2024-03-22 9:44 ` Jan Beulich 2024-03-22 13:44 ` Simone Ballarin 2024-03-22 14:12 ` Jan Beulich 2024-03-22 14:24 ` Simone Ballarin 2024-03-22 8:46 ` [XEN PATCH v2 2/2] xen: address violations of MISRA C " 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.