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

[Simone Ballarin <simone.ballarin@xxxxxxxxxxx>]

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