Re: [XEN PATCH 03/10] x86: address some violations of MISRA C Rule 20.7

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

On 2024-02-29 18:05, Jan Beulich wrote:
> On 29.02.2024 17:45, Nicola Vetrini wrote:
>> On 2024-02-29 17:37, Jan Beulich wrote:
>>> On 29.02.2024 16:27, Nicola Vetrini wrote:
>>>> --- a/xen/arch/x86/include/asm/irq.h
>>>> +++ b/xen/arch/x86/include/asm/irq.h
>>>> @@ -179,9 +179,9 @@ void cleanup_domain_irq_mapping(struct domain 
>>>> *d);
>>>>      void *__ret = radix_tree_lookup(&(d)->arch.hvm.emuirq_pirq,
>>>> emuirq);\
>>>>      __ret ? radix_tree_ptr_to_int(__ret) : IRQ_UNBOUND;
>>>>   \
>>>>  })
>>>> -#define IRQ_UNBOUND -1
>>>> -#define IRQ_PT -2
>>>> -#define IRQ_MSI_EMU -3
>>>> +#define IRQ_UNBOUND (-1)
>>>> +#define IRQ_PT      (-2)
>>>> +#define IRQ_MSI_EMU (-3)
>>>> 
>>>>  bool cpu_has_pending_apic_eoi(void);
>>>> 
>>> 
>>> I'd be happy to ack this change right away.
>>> 
>>>> --- a/xen/arch/x86/usercopy.c
>>>> +++ b/xen/arch/x86/usercopy.c
>>>> @@ -106,7 +106,7 @@ unsigned int copy_from_guest_ll(void *to, const
>>>> void __user *from, unsigned int
>>>>      return n;
>>>>  }
>>>> 
>>>> -#if GUARD(1) + 0
>>>> +#if GUARD((1)) + 0
>>> 
>>> I don't even understand the need for this one, and nothing is said in
>>> the description in that regard. Generally I'm afraid I'm averse to
>>> such (seemingly) redundant parentheses in macro invocations.
>>> 
>> 
>> It's because
>> #define UA_KEEP(args...) args
>> #define GUARD UA_KEEP
>> 
>> which would expand to #if 1 + 0, while the rule demands #if (1) + 0
>> I did note in the message after --- that I didn't wanna touch UA_KEEP 
>> so
>> I did this instead, which I'm not particularly happy about either. I 
>> can
>> remove this and deviate, there is no other issue with GUARD.
> 
> Or
> 
> #if (GUARD(1) + 0)
> 
> ? To me at least that's quite a bit less odd. But I guess that still
> wouldn't satisfy the rule. Perhaps even
> 
> #if (GUARD(1)) + 0
> 
> would be a little less odd, albeit there I'd already be on the edge.
> 

Sorry for the late reply. I'll split this in v2. Solution #2 seems ok at 
first glance.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)