[XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1

[XEN PATCH 0/2] xen: address violations of MISRA C Rule 17.1

[Simone Ballarin]

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

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

[Simone Ballarin]

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

[XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1

[Simone Ballarin]

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

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

[Jan Beulich]

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

Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1

[Jan Beulich]

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

Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1

[Stefano Stabellini]

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?

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

[Stefano Stabellini]

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?

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

[Jan Beulich]

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

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

[Stefano Stabellini]

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

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

[Simone Ballarin]

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)

Re: [XEN PATCH 2/2] xen: address violations of MISRA C Rule 17.1

[Simone Ballarin]

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)