Re: [XEN PATCH] common/sched: address a violation of MISRA C Rule 17.7

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.06.2024 02:15, victorm.lira@xxxxxxx wrote:
> From: Victor Lira <victorm.lira@xxxxxxx>
> 
> Rule 17.7: "The value returned by a function having non-void return type
> shall be used"
> 
> This patch fixes this by adding a check to the return value.
> No functional changes.
> 
> Signed-off-by: Victor Lira <victorm.lira@xxxxxxx>
> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> Cc: Dario Faggioli <dfaggioli@xxxxxxxx>
> Cc: Juergen Gross <jgross@xxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
> ---
>  xen/common/sched/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index d84b65f197..e1cd824622 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -2789,7 +2789,10 @@ static int cpu_schedule_up(unsigned int cpu)
>      BUG_ON(cpu >= NR_CPUS);
>  
>      if ( idle_vcpu[cpu] == NULL )
> -        vcpu_create(idle_vcpu[0]->domain, cpu);
> +    {
> +        if ( vcpu_create(idle_vcpu[0]->domain, cpu) == NULL )
> +            return -ENOMEM;
> +    }

First: Two such if()s want folding.

>      else
>          idle_vcpu[cpu]->sched_unit->res = sr;
>  

Then: Down from here there is

    if ( idle_vcpu[cpu] == NULL )
        return -ENOMEM;

which your change is rendering redundant for at least the vcpu_create()
path.

Finally, as we're touching error handling here (and mayby more a question
to the maintainers than to you): What about sr in the error case? It's
being allocated earlier in the function, but not freed upon error. Hmm,
looks like cpu_schedule_down() is assumed to be taking care of the case,
yet then I wonder how that can assume that get_sched_res() would return
non-NULL - afaict it may be called without cpu_schedule_up() having run
first, or with it having bailed early with -ENOMEM.

Jan