Re: [PATCH 5/5] x86: Fix missing brackets in macros

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.12.2025 10:15, Nicola Vetrini wrote:
> On 2025-12-11 09:36, Jan Beulich wrote:
>> On 10.12.2025 19:30, Andrew Cooper wrote:
>>> With the wider testing, some more violations have been spotted.  This
>>> addresses violations of Rule 20.7 which requires macro parameters to 
>>> be
>>> bracketed.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: consulting@xxxxxxxxxxx <consulting@xxxxxxxxxxx>
>>> CC: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
>>> ---
>>>  xen/arch/x86/mm/shadow/multi.c     | 2 +-
>>>  xen/arch/x86/mm/shadow/private.h   | 6 +++---
>>>  xen/drivers/passthrough/vtd/dmar.h | 2 +-
>>>  xen/include/xen/kexec.h            | 4 ++--
>>>  4 files changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/mm/shadow/multi.c 
>>> b/xen/arch/x86/mm/shadow/multi.c
>>> index 03be61e225c0..36ee6554b4c4 100644
>>> --- a/xen/arch/x86/mm/shadow/multi.c
>>> +++ b/xen/arch/x86/mm/shadow/multi.c
>>> @@ -781,7 +781,7 @@ do {                                               
>>>                      \
>>>          (_sl1e) = _sp + _i;                                           
>>>   \
>>>          if ( shadow_l1e_get_flags(*(_sl1e)) & _PAGE_PRESENT )         
>>>   \
>>>              {_code}                                                   
>>>   \
>>> -        if ( _done ) break;                                           
>>>   \
>>> +        if ( (_done) ) break;                                         
>>>   \
>>
>> I don't understand this: There are parentheses already from if() 
>> itself.
> 
> Yeah, syntactically there are, but those are parsed as part of the if, 
> rather than its condition; the AST node contained within does not have 
> parentheses around it.

I fear I don't follow. Besides us not using parentheses elsewhere when
if() is used like this macros, the point of requiring parentheses is (aiui)
to make precedence explicit. There already is no ambiguity here due to the
syntactically require parentheses in if(). Why would a rule and/or the
tool require redundant ones?

Jan