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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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)