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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> --- a/xen/arch/x86/mm/shadow/private.h
> +++ b/xen/arch/x86/mm/shadow/private.h
> @@ -636,9 +636,9 @@ prev_pinned_shadow(struct page_info *page,
>  }
>  
>  #define foreach_pinned_shadow(dom, pos, tmp)                    \
> -    for ( pos = prev_pinned_shadow(NULL, (dom));                \
> -          pos ? (tmp = prev_pinned_shadow(pos, (dom)), 1) : 0;  \
> -          pos = tmp )
> +    for ( (pos) = prev_pinned_shadow(NULL, dom);                \
> +          (pos) ? (tmp = prev_pinned_shadow(pos, dom), 1) : 0;  \
> +          (pos) = tmp )

What about tmp (twice)?

Jan