Re: [XEN PATCH v3 1/2] x86/vmsi: rename variables to address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 08/08/2023 15:22, Jan Beulich wrote:
> On 08.08.2023 14:22, Nicola Vetrini wrote:
> > The local variables 'irq_desc' shadow the homonymous global variable,
> > declared in 'xen/arch/x86/include/asm/irq.h', therefore they are 
> > renamed
> > 'irqd' for consistency with ARM code. Other variables of the same type
> > in the file are also renamed 'irqd' for consistency.
> I'm pretty sure I pointed out that Arm uses a mix of "desc" and "irqd".
> So "consistency with ARM code" doesn't ...
>
> > --- a/xen/arch/x86/hvm/vmsi.c
> > +++ b/xen/arch/x86/hvm/vmsi.c
> > @@ -281,7 +281,7 @@ static int msixtbl_write(struct vcpu *v, unsigned 
> > long address,
> >      unsigned int nr_entry, index;
> >      int r = X86EMUL_UNHANDLEABLE;
> >      unsigned long flags;
> > -    struct irq_desc *desc;
> > +    struct irq_desc *irqd;
> ... require e.g. this rename. As mentioned before: Let's limit code
> churn where possible, and where going beyond what's strictly necessary
> isn't otherwise useful; there's already enough of it with all these not
> really helpful Misra changes.
>
> > @@ -462,7 +462,7 @@ static void del_msixtbl_entry(struct msixtbl_entry 
> > *entry)
> >  int msixtbl_pt_register(struct domain *d, struct pirq *pirq, uint64_t 
> > gtable)
> >  {
> > -    struct irq_desc *irq_desc;
> > +    struct irq_desc *irqd;
> This one indeed wants renaming, but then - consistent within the file -
> to "desc". At least that's my view.
>
> Jan
Well, but having

struct irq_desc *desc;
struct msi_desc *msi_desc;

and then using them both within the function doesn't seem that readable, 
but if you
prefer "desc" I have no objection (just two local variables that need to 
be changed).
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)