Re: [XEN PATCH 2/3] xen/arm: irq: address violations of MISRA C: 2012 Rules 8.2 and 8.3

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 24 Jul 2023, Federico Serafini wrote:
> Give a name to unnamed parameters thus addressing violations of
> MISRA C:2012 Rule 8.2 ("Function types shall be in prototype form with
> named parameters").
> Keep consistency between parameter names and types used in function
> declarations and the ones used in the corresponding function
> definitions, thus addressing violations of MISRA C:2012 Rule 8.3
> ("All declarations of an object or function shall use the same names
> and type qualifiers").
> 
> No functional changes.
> 
> Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> ---
>  xen/arch/arm/irq.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/irq.c b/xen/arch/arm/irq.c
> index 16e56f8945..335e06a2a7 100644
> --- a/xen/arch/arm/irq.c
> +++ b/xen/arch/arm/irq.c
> @@ -58,7 +58,7 @@ hw_irq_controller no_irq_type = {
>  static irq_desc_t irq_desc[NR_IRQS];
>  static DEFINE_PER_CPU(irq_desc_t[NR_LOCAL_IRQS], local_irq_desc);
>  
> -irq_desc_t *__irq_to_desc(int irq)
> +struct irq_desc *__irq_to_desc(int irq)
>  {
>      if ( irq < NR_LOCAL_IRQS )
>          return &this_cpu(local_irq_desc)[irq];
> @@ -182,7 +182,8 @@ void irq_set_affinity(struct irq_desc *desc, const 
> cpumask_t *cpu_mask)
>  }
>  
>  int request_irq(unsigned int irq, unsigned int irqflags,
> -                void (*handler)(int, void *, struct cpu_user_regs *),
> +                void (*handler)(int irq, void *dev_id,
> +                                struct cpu_user_regs *regs),

We have an inconsistency where the handler functions on x86 typically
call it void *data, while on arm they typically use void *dev_id
(see xen/arch/x86/irq.c:request_irq and
xen/arch/x86/hpet.c:hpet_interrupt_handler). I think we should be
consistent. Or, if this is not a MISRA requirement because this is just
a function pointer rather than a proper function, then I would leave it
alone.


>                  const char *devname, void *dev_id)
>  {
>      struct irqaction *action;
> @@ -617,7 +618,7 @@ void pirq_guest_unbind(struct domain *d, struct pirq 
> *pirq)
>      BUG();
>  }
>  
> -void pirq_set_affinity(struct domain *d, int pirq, const cpumask_t *mask)
> +void pirq_set_affinity(struct domain *d, int irq, const cpumask_t *mask)

I think we should leave it as is because there is also the x86
implementation of pirq_set_affinity that uses int pirq as parameter. It
is not a good idea to introduce inconsistencies between the x86 and the
ARM versions of the same function.