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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxxxx>


Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>

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
>