| Message ID | dc3c8f2913a9efd54767828bf11fc5767afbbfa5.1711096695.git.simone.ballarin@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: address violations of MISRA C Rule 17.1 | expand |
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
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.
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
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 >
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;
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(+)