Re: [XEN PATCH 2/2] xen/cpu: address MISRA C Rule 17.7

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 23 Feb 2024, Nicola Vetrini wrote:
> Refactor cpu_notifier_call_chain into two functions:
> - the variant that is allowed to fail loses the nofail flag
> - the variant that shouldn't fail is encapsulated in a call
>   to the failing variant, with an additional check.
> 
> This prevents uses of the function that are not supposed to
> fail from ignoring the return value, thus violating Rule 17.7:
> "The value returned by a function having non-void return type shall
> be used".
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
>  xen/common/cpu.c | 31 +++++++++++++++++++------------
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/common/cpu.c b/xen/common/cpu.c
> index 8709db4d2957..0b7cf54c4264 100644
> --- a/xen/common/cpu.c
> +++ b/xen/common/cpu.c
> @@ -78,20 +78,27 @@ void __init register_cpu_notifier(struct notifier_block 
> *nb)
>  }
>  
>  static int cpu_notifier_call_chain(unsigned int cpu, unsigned long action,
> -                                   struct notifier_block **nb, bool nofail)
> +                                   struct notifier_block **nb)
>  {
>      void *hcpu = (void *)(long)cpu;
>      int notifier_rc = notifier_call_chain(&cpu_chain, action, hcpu, nb);
>      int ret =  notifier_to_errno(notifier_rc);
>  
> -    BUG_ON(ret && nofail);
> -
>      return ret;
>  }
>  
> +static void cpu_notifier_call_chain_nofail(unsigned int cpu,
> +                                           unsigned long action,
> +                                           struct notifier_block **nb)
> +{
> +    int ret = cpu_notifier_call_chain(cpu, action, nb);
> +
> +    BUG_ON(ret);
> +}
> +
>  static void cf_check _take_cpu_down(void *unused)
>  {
> -    cpu_notifier_call_chain(smp_processor_id(), CPU_DYING, NULL, true);
> +    cpu_notifier_call_chain_nofail(smp_processor_id(), CPU_DYING, NULL);
>      __cpu_disable();
>  }
>  
> @@ -116,7 +123,7 @@ int cpu_down(unsigned int cpu)
>      if ( !cpu_online(cpu) )
>          goto out;
>  
> -    err = cpu_notifier_call_chain(cpu, CPU_DOWN_PREPARE, &nb, false);
> +    err = cpu_notifier_call_chain(cpu, CPU_DOWN_PREPARE, &nb);
>      if ( err )
>          goto fail;
>  
> @@ -129,14 +136,14 @@ int cpu_down(unsigned int cpu)
>      err = cpu_online(cpu);
>      BUG_ON(err);
>  
> -    cpu_notifier_call_chain(cpu, CPU_DEAD, NULL, true);
> +    cpu_notifier_call_chain_nofail(cpu, CPU_DEAD, NULL);
>  
>      send_global_virq(VIRQ_PCPU_STATE);
>      cpu_hotplug_done();
>      return 0;
>  
>   fail:
> -    cpu_notifier_call_chain(cpu, CPU_DOWN_FAILED, &nb, true);
> +    cpu_notifier_call_chain_nofail(cpu, CPU_DOWN_FAILED, &nb);
>   out:
>      cpu_hotplug_done();
>      return err;
> @@ -157,7 +164,7 @@ int cpu_up(unsigned int cpu)
>      if ( cpu_online(cpu) )
>          goto out;
>  
> -    err = cpu_notifier_call_chain(cpu, CPU_UP_PREPARE, &nb, false);
> +    err = cpu_notifier_call_chain(cpu, CPU_UP_PREPARE, &nb);
>      if ( err )
>          goto fail;
>  
> @@ -165,7 +172,7 @@ int cpu_up(unsigned int cpu)
>      if ( err < 0 )
>          goto fail;
>  
> -    cpu_notifier_call_chain(cpu, CPU_ONLINE, NULL, true);
> +    cpu_notifier_call_chain_nofail(cpu, CPU_ONLINE, NULL);
>  
>      send_global_virq(VIRQ_PCPU_STATE);
>  
> @@ -173,7 +180,7 @@ int cpu_up(unsigned int cpu)
>      return 0;
>  
>   fail:
> -    cpu_notifier_call_chain(cpu, CPU_UP_CANCELED, &nb, true);
> +    cpu_notifier_call_chain_nofail(cpu, CPU_UP_CANCELED, &nb);
>   out:
>      cpu_hotplug_done();
>      return err;
> @@ -181,7 +188,7 @@ int cpu_up(unsigned int cpu)
>  
>  void notify_cpu_starting(unsigned int cpu)
>  {
> -    cpu_notifier_call_chain(cpu, CPU_STARTING, NULL, true);
> +    cpu_notifier_call_chain_nofail(cpu, CPU_STARTING, NULL);
>  }
>  
>  static cpumask_t frozen_cpus;
> @@ -237,7 +244,7 @@ void enable_nonboot_cpus(void)
>      }
>  
>      for_each_cpu ( cpu, &frozen_cpus )
> -        cpu_notifier_call_chain(cpu, CPU_RESUME_FAILED, NULL, true);
> +        cpu_notifier_call_chain_nofail(cpu, CPU_RESUME_FAILED, NULL);
>  
>      cpumask_clear(&frozen_cpus);
>  }
> -- 
> 2.34.1
>