Mailing List Archive: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `' shall not be used"

[XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used"

Mar 28, 2024, 3:29 AM

Post #1 of 6 (36 views)

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 v3:
- rebase: change SAF-3-safe to SAF-4-safe.
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 d361d0e65c..fe2bc18509 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -36,6 +36,14 @@
},
{
"id": "SAF-4-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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */
va_list args;

ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
--
2.34.1

Re: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" [ In reply to ]

jbeulich at suse

Mar 28, 2024, 3:31 AM

Post #2 of 6 (36 views)

Permalink

On 28.03.2024 11:29, Simone Ballarin wrote:
> The Xen community wants to avoid using variadic functions except for
> specific circumstances where it feels appropriate by strict code review.

In the title, s/20.7/17.1/ I suppose?

Jan

> 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 v3:
> - rebase: change SAF-3-safe to SAF-4-safe.
> 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 d361d0e65c..fe2bc18509 100644
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -36,6 +36,14 @@
> },
> {
> "id": "SAF-4-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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */
> va_list args;
>
> ASSERT(nr <= ARRAY_SIZE(mcs->call.args));

Re: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" [ In reply to ]

simone.ballarin at bugseng

Mar 28, 2024, 3:50 AM

Post #3 of 6 (36 views)

Permalink

On 28/03/24 11:31, Jan Beulich wrote:
> On 28.03.2024 11:29, Simone Ballarin wrote:
>> The Xen community wants to avoid using variadic functions except for
>> specific circumstances where it feels appropriate by strict code review.
>
> In the title, s/20.7/17.1/ I suppose?
>
> Jan
>
>> 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 v3:
>> - rebase: change SAF-3-safe to SAF-4-safe.
>> 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 d361d0e65c..fe2bc18509 100644
>> --- a/docs/misra/safe.json
>> +++ b/docs/misra/safe.json
>> @@ -36,6 +36,14 @@
>> },
>> {
>> "id": "SAF-4-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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */
>> va_list args;
>>
>> ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
>
>

Yes, sorry.

--
Simone Ballarin, M.Sc.

Field Application Engineer, BUGSENG (https://bugseng.com)

Re: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" [ In reply to ]

sstabellini at kernel

Apr 4, 2024, 5:27 PM

Post #4 of 6 (25 views)

Permalink

On Thu, 28 Mar 2024, Simone Ballarin wrote:
> 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>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

I fixed the title on commit

> ---
> Changes in v3:
> - rebase: change SAF-3-safe to SAF-4-safe.
> 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 d361d0e65c..fe2bc18509 100644
> --- a/docs/misra/safe.json
> +++ b/docs/misra/safe.json
> @@ -36,6 +36,14 @@
> },
> {
> "id": "SAF-4-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-5-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 f38cb5e04c..34cbfe699a 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-4-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..133e9f221c 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-4-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-4-safe allowed variadic function */
> va_list args;
>
> ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
> --
> 2.34.1
>

Re: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" [ In reply to ]

jbeulich at suse

Apr 4, 2024, 11:45 PM

Post #5 of 6 (25 views)

Permalink

On 05.04.2024 02:27, Stefano Stabellini wrote:
> On Thu, 28 Mar 2024, Simone Ballarin wrote:
>> 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>
>
>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
>
> I fixed the title on commit

Did you forget to push then? I don't see this change in the tree, yet.

Jan

Re: [XEN PATCH v3 2/2] MISRA C Rule 20.7 states: "The features of `<stdarg.h>' shall not be used" [ In reply to ]

sstabellini at kernel

Apr 5, 2024, 11:16 AM

Post #6 of 6 (25 views)

Permalink

On Fri, 5 Apr 2024, Jan Beulich wrote:
> On 05.04.2024 02:27, Stefano Stabellini wrote:
> > On Thu, 28 Mar 2024, Simone Ballarin wrote:
> >> 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>
> >
> >
> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> >
> > I fixed the title on commit
>
> Did you forget to push then? I don't see this change in the tree, yet.

Yep, thanks!