[PATCH 11/12] xen: Bracket uses of macro parameters

Andrew Cooper posted 12 patches 1 week, 3 days ago
[PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Andrew Cooper 1 week, 3 days ago
Fixing Rule 20.7 violations.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Bertrand Marquis <bertrand.marquis@arm.com>
CC: Michal Orzel <michal.orzel@amd.com>
CC: consulting@bugseng.com <consulting@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/arm/pci/pci-access.c       |  6 +++---
 xen/arch/x86/include/asm/bootinfo.h |  2 +-
 xen/arch/x86/mm/shadow/multi.c      |  2 +-
 xen/common/kimage.c                 | 11 ++++++-----
 xen/common/livepatch.c              | 11 ++++++-----
 xen/crypto/vmac.c                   |  8 ++++----
 xen/include/xen/libfdt/libfdt.h     |  6 +++---
 7 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/xen/arch/arm/pci/pci-access.c b/xen/arch/arm/pci/pci-access.c
index 2b23d552bbfb..7bd6877596d4 100644
--- a/xen/arch/arm/pci/pci-access.c
+++ b/xen/arch/arm/pci/pci-access.c
@@ -16,7 +16,7 @@
 #include <asm/io.h>
 
 #define INVALID_VALUE (~0U)
-#define PCI_ERR_VALUE(len) GENMASK(0, len * 8)
+#define PCI_ERR_VALUE(len) GENMASK(0, (len) * 8)
 
 static const struct pci_ops *get_ops(struct pci_host_bridge *bridge,
                                      pci_sbdf_t sbdf)
@@ -139,14 +139,14 @@ static void pci_config_write(pci_sbdf_t sbdf, unsigned int reg,
     void pci_conf_write##size(pci_sbdf_t sbdf,              \
                               unsigned int reg, type data)  \
 {                                                           \
-    pci_config_write(sbdf, reg, size / 8, data);            \
+    pci_config_write(sbdf, reg, (size) / 8, data);          \
 }
 
 #define PCI_OP_READ(size, type)                             \
     type pci_conf_read##size(pci_sbdf_t sbdf,               \
                               unsigned int reg)             \
 {                                                           \
-    return pci_config_read(sbdf, reg, size / 8);            \
+    return pci_config_read(sbdf, reg, (size) / 8);          \
 }
 
 PCI_OP_READ(8, uint8_t)
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 4f2cc5906e59..b3a6b2acf3ac 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -75,7 +75,7 @@ static inline unsigned int __init next_boot_module_index(
 #define for_each_boot_module_by_type(i, b, t)           \
     for ( (i) = first_boot_module_index(b, t);          \
           (i) <= (b)->nr_modules;                       \
-          (i) = next_boot_module_index(b, t, i + 1) )
+          (i) = next_boot_module_index(b, t, (i) + 1) )
 
 #endif /* X86_BOOTINFO_H */
 
diff --git a/xen/arch/x86/mm/shadow/multi.c b/xen/arch/x86/mm/shadow/multi.c
index 3bbaceec1287..9de60720db20 100644
--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -793,7 +793,7 @@ do {                                                                    \
 do {                                                                    \
     int __done = 0;                                                     \
     _FOREACH_PRESENT_L1E(_sl1mfn, _sl1e, _gl1p,                         \
-                         ({ (__done = _done); }), _code);               \
+                         ({ (__done = (_done)); }), _code);             \
     _sl1mfn = sh_next_page(_sl1mfn);                                    \
     if ( !__done )                                                      \
         _FOREACH_PRESENT_L1E(_sl1mfn, _sl1e, _gl1p, _done, _code);      \
diff --git a/xen/common/kimage.c b/xen/common/kimage.c
index e1aec5a18a54..972301368443 100644
--- a/xen/common/kimage.c
+++ b/xen/common/kimage.c
@@ -491,11 +491,12 @@ static void kimage_terminate(struct kexec_image *image)
  * Call unmap_domain_page(ptr) after the loop exits.
  */
 #define for_each_kimage_entry(image, ptr, entry)                        \
-    for ( ptr = map_domain_page(_mfn(paddr_to_pfn(image->head)));       \
-          (entry = *ptr) && !(entry & IND_DONE);                        \
-          ptr = (entry & IND_INDIRECTION) ?                             \
-              (unmap_domain_page(ptr), map_domain_page(_mfn(paddr_to_pfn(entry)))) \
-              : ptr + 1 )
+    for ( (ptr) = map_domain_page(_mfn(paddr_to_pfn((image)->head)));   \
+          ((entry) = *(ptr)) && !((entry) & IND_DONE);                  \
+          (ptr) = (((entry) & IND_INDIRECTION)                          \
+                   ? (unmap_domain_page(ptr),                           \
+                      map_domain_page(_mfn(paddr_to_pfn(entry))))       \
+                   : (ptr) + 1) )
 
 static void kimage_free_entry(kimage_entry_t entry)
 {
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 7446533c8cfb..63473925cafb 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -664,9 +664,10 @@ static inline int livepatch_check_expectations(const struct payload *payload)
     const struct livepatch_elf_sec *__sec = livepatch_elf_sec_by_name(elf, section_name); \
     if ( !__sec )                                                                         \
         break;                                                                            \
-    if ( !section_ok(elf, __sec, sizeof(*hook)) || __sec->sec->sh_size != sizeof(*hook) ) \
+    if ( !section_ok(elf, __sec, sizeof(*(hook))) ||                                      \
+         __sec->sec->sh_size != sizeof(*(hook)) )                                         \
         return -EINVAL;                                                                   \
-    hook = __sec->addr;                                                                   \
+    (hook) = __sec->addr;                                                                 \
 } while (0)
 
 /*
@@ -678,10 +679,10 @@ static inline int livepatch_check_expectations(const struct payload *payload)
     const struct livepatch_elf_sec *__sec = livepatch_elf_sec_by_name(elf, section_name); \
     if ( !__sec )                                                                         \
         break;                                                                            \
-    if ( !section_ok(elf, __sec, sizeof(*hook)) )                                         \
+    if ( !section_ok(elf, __sec, sizeof(*(hook))) )                                       \
         return -EINVAL;                                                                   \
-    hook = __sec->addr;                                                                   \
-    nhooks = __sec->sec->sh_size / sizeof(*hook);                                         \
+    (hook) = __sec->addr;                                                                 \
+    (nhooks) = __sec->sec->sh_size / sizeof(*(hook));                                     \
 } while (0)
 
 static int prepare_payload(struct payload *payload,
diff --git a/xen/crypto/vmac.c b/xen/crypto/vmac.c
index c9914d2c7c1d..c7aa1e9be3b9 100644
--- a/xen/crypto/vmac.c
+++ b/xen/crypto/vmac.c
@@ -203,7 +203,7 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 #define nh_16(mp, kp, nw, rh, rl)                                            \
 {   int i; uint64_t th, tl;                                                  \
     rh = rl = 0;                                                             \
-    for (i = 0; i < nw; i+= 2) {                                             \
+    for (i = 0; i < (nw); i+= 2) {                                           \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  ],get64PE((mp)+i+1)+(kp)[i+1]);\
         ADD128(rh,rl,th,tl);                                                 \
     }                                                                        \
@@ -211,7 +211,7 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 #define nh_16_2(mp, kp, nw, rh, rl, rh1, rl1)                                \
 {   int i; uint64_t th, tl;                                                  \
     rh1 = rl1 = rh = rl = 0;                                                 \
-    for (i = 0; i < nw; i+= 2) {                                             \
+    for (i = 0; i < (nw); i+= 2) {                                           \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  ],get64PE((mp)+i+1)+(kp)[i+1]);\
         ADD128(rh,rl,th,tl);                                                 \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i+2],get64PE((mp)+i+1)+(kp)[i+3]);\
@@ -223,7 +223,7 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 #define nh_vmac_nhbytes(mp, kp, nw, rh, rl)                                  \
 {   int i; uint64_t th, tl;                                                  \
     rh = rl = 0;                                                             \
-    for (i = 0; i < nw; i+= 8) {                                             \
+    for (i = 0; i < (nw); i+= 8) {                                           \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  ],get64PE((mp)+i+1)+(kp)[i+1]);\
         ADD128(rh,rl,th,tl);                                                 \
         MUL64(th,tl,get64PE((mp)+i+2)+(kp)[i+2],get64PE((mp)+i+3)+(kp)[i+3]);\
@@ -237,7 +237,7 @@ const uint64_t mpoly = UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
 #define nh_vmac_nhbytes_2(mp, kp, nw, rh, rl, rh1, rl1)                      \
 {   int i; uint64_t th, tl;                                                  \
     rh1 = rl1 = rh = rl = 0;                                                 \
-    for (i = 0; i < nw; i+= 8) {                                             \
+    for (i = 0; i < (nw); i+= 8) {                                           \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  ],get64PE((mp)+i+1)+(kp)[i+1]);\
         ADD128(rh,rl,th,tl);                                                 \
         MUL64(th,tl,get64PE((mp)+i  )+(kp)[i+2],get64PE((mp)+i+1)+(kp)[i+3]);\
diff --git a/xen/include/xen/libfdt/libfdt.h b/xen/include/xen/libfdt/libfdt.h
index fabddbee8cec..1b09bba28eea 100644
--- a/xen/include/xen/libfdt/libfdt.h
+++ b/xen/include/xen/libfdt/libfdt.h
@@ -230,9 +230,9 @@ int fdt_next_subnode(const void *fdt, int offset);
  * literal.
  */
 #define fdt_for_each_subnode(node, fdt, parent)		\
-	for (node = fdt_first_subnode(fdt, parent);	\
-	     node >= 0;					\
-	     node = fdt_next_subnode(fdt, node))
+	for ((node) = fdt_first_subnode(fdt, parent);	\
+	     (node) >= 0;				\
+	     (node) = fdt_next_subnode(fdt, node))
 
 /**********************************************************************/
 /* General functions                                                  */
-- 
2.39.5


Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Jan Beulich 1 week ago
On 20.02.2026 22:46, Andrew Cooper wrote:
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -793,7 +793,7 @@ do {                                                                    \
>  do {                                                                    \
>      int __done = 0;                                                     \
>      _FOREACH_PRESENT_L1E(_sl1mfn, _sl1e, _gl1p,                         \
> -                         ({ (__done = _done); }), _code);               \
> +                         ({ (__done = (_done)); }), _code);             \

Could I talk you into dropping the unnecessary pair of parentheses (around the
assignment) in exchange?

Jan
Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Orzel, Michal 1 week ago

On 20/02/2026 22:46, Andrew Cooper wrote:
> Fixing Rule 20.7 violations.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Nicola Vetrini 1 week, 3 days ago
On 2026-02-20 22:46, Andrew Cooper wrote:
> Fixing Rule 20.7 violations.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>


Two minor nits below

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien@xen.org>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis <bertrand.marquis@arm.com>
> CC: Michal Orzel <michal.orzel@amd.com>
> CC: consulting@bugseng.com <consulting@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>  xen/arch/arm/pci/pci-access.c       |  6 +++---
>  xen/arch/x86/include/asm/bootinfo.h |  2 +-
>  xen/arch/x86/mm/shadow/multi.c      |  2 +-
>  xen/common/kimage.c                 | 11 ++++++-----
>  xen/common/livepatch.c              | 11 ++++++-----
>  xen/crypto/vmac.c                   |  8 ++++----
>  xen/include/xen/libfdt/libfdt.h     |  6 +++---
>  7 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/xen/arch/arm/pci/pci-access.c 
> b/xen/arch/arm/pci/pci-access.c
> index 2b23d552bbfb..7bd6877596d4 100644
> --- a/xen/arch/arm/pci/pci-access.c
> +++ b/xen/arch/arm/pci/pci-access.c
> @@ -16,7 +16,7 @@
>  #include <asm/io.h>
> 
>  #define INVALID_VALUE (~0U)
> -#define PCI_ERR_VALUE(len) GENMASK(0, len * 8)
> +#define PCI_ERR_VALUE(len) GENMASK(0, (len) * 8)
> 
>  static const struct pci_ops *get_ops(struct pci_host_bridge *bridge,
>                                       pci_sbdf_t sbdf)
> @@ -139,14 +139,14 @@ static void pci_config_write(pci_sbdf_t sbdf, 
> unsigned int reg,
>      void pci_conf_write##size(pci_sbdf_t sbdf,              \
>                                unsigned int reg, type data)  \
>  {                                                           \
> -    pci_config_write(sbdf, reg, size / 8, data);            \
> +    pci_config_write(sbdf, reg, (size) / 8, data);          \
>  }
> 
>  #define PCI_OP_READ(size, type)                             \
>      type pci_conf_read##size(pci_sbdf_t sbdf,               \
>                                unsigned int reg)             \
>  {                                                           \
> -    return pci_config_read(sbdf, reg, size / 8);            \
> +    return pci_config_read(sbdf, reg, (size) / 8);          \
>  }
> 
>  PCI_OP_READ(8, uint8_t)
> diff --git a/xen/arch/x86/include/asm/bootinfo.h 
> b/xen/arch/x86/include/asm/bootinfo.h
> index 4f2cc5906e59..b3a6b2acf3ac 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -75,7 +75,7 @@ static inline unsigned int __init 
> next_boot_module_index(
>  #define for_each_boot_module_by_type(i, b, t)           \
>      for ( (i) = first_boot_module_index(b, t);          \
>            (i) <= (b)->nr_modules;                       \
> -          (i) = next_boot_module_index(b, t, i + 1) )
> +          (i) = next_boot_module_index(b, t, (i) + 1) )
> 
>  #endif /* X86_BOOTINFO_H */
> 
> diff --git a/xen/arch/x86/mm/shadow/multi.c 
> b/xen/arch/x86/mm/shadow/multi.c
> index 3bbaceec1287..9de60720db20 100644
> --- a/xen/arch/x86/mm/shadow/multi.c
> +++ b/xen/arch/x86/mm/shadow/multi.c
> @@ -793,7 +793,7 @@ do {                                                
>                     \
>  do {                                                                   
>  \
>      int __done = 0;                                                    
>  \
>      _FOREACH_PRESENT_L1E(_sl1mfn, _sl1e, _gl1p,                        
>  \
> -                         ({ (__done = _done); }), _code);              
>  \
> +                         ({ (__done = (_done)); }), _code);            
>  \
>      _sl1mfn = sh_next_page(_sl1mfn);                                   
>  \
>      if ( !__done )                                                     
>  \
>          _FOREACH_PRESENT_L1E(_sl1mfn, _sl1e, _gl1p, _done, _code);     
>  \
> diff --git a/xen/common/kimage.c b/xen/common/kimage.c
> index e1aec5a18a54..972301368443 100644
> --- a/xen/common/kimage.c
> +++ b/xen/common/kimage.c
> @@ -491,11 +491,12 @@ static void kimage_terminate(struct kexec_image 
> *image)
>   * Call unmap_domain_page(ptr) after the loop exits.
>   */
>  #define for_each_kimage_entry(image, ptr, entry)                       
>  \
> -    for ( ptr = map_domain_page(_mfn(paddr_to_pfn(image->head)));      
>  \
> -          (entry = *ptr) && !(entry & IND_DONE);                       
>  \
> -          ptr = (entry & IND_INDIRECTION) ?                            
>  \
> -              (unmap_domain_page(ptr), 
> map_domain_page(_mfn(paddr_to_pfn(entry)))) \
> -              : ptr + 1 )
> +    for ( (ptr) = map_domain_page(_mfn(paddr_to_pfn((image)->head)));  
>  \
> +          ((entry) = *(ptr)) && !((entry) & IND_DONE);                 
>  \
> +          (ptr) = (((entry) & IND_INDIRECTION)                         
>  \
> +                   ? (unmap_domain_page(ptr),                          
>  \
> +                      map_domain_page(_mfn(paddr_to_pfn(entry))))      
>  \
> +                   : (ptr) + 1) )
> 
>  static void kimage_free_entry(kimage_entry_t entry)
>  {
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 7446533c8cfb..63473925cafb 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -664,9 +664,10 @@ static inline int 
> livepatch_check_expectations(const struct payload *payload)
>      const struct livepatch_elf_sec *__sec = 
> livepatch_elf_sec_by_name(elf, section_name); \
>      if ( !__sec )                                                      
>                    \
>          break;                                                         
>                    \
> -    if ( !section_ok(elf, __sec, sizeof(*hook)) || __sec->sec->sh_size 
> != sizeof(*hook) ) \
> +    if ( !section_ok(elf, __sec, sizeof(*(hook))) ||                   
>                    \
> +         __sec->sec->sh_size != sizeof(*(hook)) )                      
>                    \
>          return -EINVAL;                                                
>                    \
> -    hook = __sec->addr;                                                
>                    \
> +    (hook) = __sec->addr;                                              
>                    \

This is not strictly needed, if not for consistency

>  } while (0)
> 
>  /*
> @@ -678,10 +679,10 @@ static inline int 
> livepatch_check_expectations(const struct payload *payload)
>      const struct livepatch_elf_sec *__sec = 
> livepatch_elf_sec_by_name(elf, section_name); \
>      if ( !__sec )                                                      
>                    \
>          break;                                                         
>                    \
> -    if ( !section_ok(elf, __sec, sizeof(*hook)) )                      
>                    \
> +    if ( !section_ok(elf, __sec, sizeof(*(hook))) )                    
>                    \
>          return -EINVAL;                                                
>                    \
> -    hook = __sec->addr;                                                
>                    \
> -    nhooks = __sec->sec->sh_size / sizeof(*hook);                      
>                    \
> +    (hook) = __sec->addr;                                              
>                    \
> +    (nhooks) = __sec->sec->sh_size / sizeof(*(hook));                  
>                    \

same as above

>  } while (0)
> 
>  static int prepare_payload(struct payload *payload,
> diff --git a/xen/crypto/vmac.c b/xen/crypto/vmac.c
> index c9914d2c7c1d..c7aa1e9be3b9 100644
> --- a/xen/crypto/vmac.c
> +++ b/xen/crypto/vmac.c
> @@ -203,7 +203,7 @@ const uint64_t mpoly = 
> UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
>  #define nh_16(mp, kp, nw, rh, rl)                                      
>       \
>  {   int i; uint64_t th, tl;                                            
>       \
>      rh = rl = 0;                                                       
>       \
> -    for (i = 0; i < nw; i+= 2) {                                       
>       \
> +    for (i = 0; i < (nw); i+= 2) {                                     
>       \
>          MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  
> ],get64PE((mp)+i+1)+(kp)[i+1]);\
>          ADD128(rh,rl,th,tl);                                           
>       \
>      }                                                                  
>       \
> @@ -211,7 +211,7 @@ const uint64_t mpoly = 
> UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
>  #define nh_16_2(mp, kp, nw, rh, rl, rh1, rl1)                          
>       \
>  {   int i; uint64_t th, tl;                                            
>       \
>      rh1 = rl1 = rh = rl = 0;                                           
>       \
> -    for (i = 0; i < nw; i+= 2) {                                       
>       \
> +    for (i = 0; i < (nw); i+= 2) {                                     
>       \
>          MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  
> ],get64PE((mp)+i+1)+(kp)[i+1]);\
>          ADD128(rh,rl,th,tl);                                           
>       \
>          MUL64(th,tl,get64PE((mp)+i  
> )+(kp)[i+2],get64PE((mp)+i+1)+(kp)[i+3]);\
> @@ -223,7 +223,7 @@ const uint64_t mpoly = 
> UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
>  #define nh_vmac_nhbytes(mp, kp, nw, rh, rl)                            
>       \
>  {   int i; uint64_t th, tl;                                            
>       \
>      rh = rl = 0;                                                       
>       \
> -    for (i = 0; i < nw; i+= 8) {                                       
>       \
> +    for (i = 0; i < (nw); i+= 8) {                                     
>       \
>          MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  
> ],get64PE((mp)+i+1)+(kp)[i+1]);\
>          ADD128(rh,rl,th,tl);                                           
>       \
>          
> MUL64(th,tl,get64PE((mp)+i+2)+(kp)[i+2],get64PE((mp)+i+3)+(kp)[i+3]);\
> @@ -237,7 +237,7 @@ const uint64_t mpoly = 
> UINT64_C(0x1fffffff1fffffff);  /* Poly key mask     */
>  #define nh_vmac_nhbytes_2(mp, kp, nw, rh, rl, rh1, rl1)                
>       \
>  {   int i; uint64_t th, tl;                                            
>       \
>      rh1 = rl1 = rh = rl = 0;                                           
>       \
> -    for (i = 0; i < nw; i+= 8) {                                       
>       \
> +    for (i = 0; i < (nw); i+= 8) {                                     
>       \
>          MUL64(th,tl,get64PE((mp)+i  )+(kp)[i  
> ],get64PE((mp)+i+1)+(kp)[i+1]);\
>          ADD128(rh,rl,th,tl);                                           
>       \
>          MUL64(th,tl,get64PE((mp)+i  
> )+(kp)[i+2],get64PE((mp)+i+1)+(kp)[i+3]);\
> diff --git a/xen/include/xen/libfdt/libfdt.h 
> b/xen/include/xen/libfdt/libfdt.h
> index fabddbee8cec..1b09bba28eea 100644
> --- a/xen/include/xen/libfdt/libfdt.h
> +++ b/xen/include/xen/libfdt/libfdt.h
> @@ -230,9 +230,9 @@ int fdt_next_subnode(const void *fdt, int offset);
>   * literal.
>   */
>  #define fdt_for_each_subnode(node, fdt, parent)		\
> -	for (node = fdt_first_subnode(fdt, parent);	\
> -	     node >= 0;					\
> -	     node = fdt_next_subnode(fdt, node))
> +	for ((node) = fdt_first_subnode(fdt, parent);	\
> +	     (node) >= 0;				\
> +	     (node) = fdt_next_subnode(fdt, node))
> 
>  
> /**********************************************************************/
>  /* General functions                                                  
> */

-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Andrew Cooper 5 days, 11 hours ago
On 20/02/2026 10:45 pm, Nicola Vetrini wrote:
> On 2026-02-20 22:46, Andrew Cooper wrote:
>> Fixing Rule 20.7 violations.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Thanks.

>> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
>> index 7446533c8cfb..63473925cafb 100644
>> --- a/xen/common/livepatch.c
>> +++ b/xen/common/livepatch.c
>> @@ -664,9 +664,10 @@ static inline int
>> livepatch_check_expectations(const struct payload *payload)
>>      const struct livepatch_elf_sec *__sec =
>> livepatch_elf_sec_by_name(elf, section_name); \
>>      if ( !__sec
>> )                                                     
>>                    \
>>         
>> break;                                                        
>>                    \
>> -    if ( !section_ok(elf, __sec, sizeof(*hook)) ||
>> __sec->sec->sh_size != sizeof(*hook) ) \
>> +    if ( !section_ok(elf, __sec, sizeof(*(hook)))
>> ||                                      \
>> +         __sec->sec->sh_size != sizeof(*(hook))
>> )                                         \
>>          return
>> -EINVAL;                                               
>>                    \
>> -    hook =
>> __sec->addr;                                               
>>                    \
>> +    (hook) =
>> __sec->addr;                                             
>>                    \
>
> This is not strictly needed, if not for consistency.

That explains why my first try didn't succeed.  But, why is it that only
MISRA only cares about bracketing for rvalues, not lvalues ?

~Andrew

Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Nicola Vetrini 5 days, 10 hours ago
On 2026-02-25 17:05, Andrew Cooper wrote:
> On 20/02/2026 10:45 pm, Nicola Vetrini wrote:
>> On 2026-02-20 22:46, Andrew Cooper wrote:
>>> Fixing Rule 20.7 violations.
>>> 
>>> No functional change.
>>> 
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> Thanks.
> 
>>> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
>>> index 7446533c8cfb..63473925cafb 100644
>>> --- a/xen/common/livepatch.c
>>> +++ b/xen/common/livepatch.c
>>> @@ -664,9 +664,10 @@ static inline int
>>> livepatch_check_expectations(const struct payload *payload)
>>>      const struct livepatch_elf_sec *__sec =
>>> livepatch_elf_sec_by_name(elf, section_name); \
>>>      if ( !__sec
>>> )                                                     
>>>                    \
>>>         
>>> break;                                                        
>>>                    \
>>> -    if ( !section_ok(elf, __sec, sizeof(*hook)) ||
>>> __sec->sec->sh_size != sizeof(*hook) ) \
>>> +    if ( !section_ok(elf, __sec, sizeof(*(hook)))
>>> ||                                      \
>>> +         __sec->sec->sh_size != sizeof(*(hook))
>>> )                                         \
>>>          return
>>> -EINVAL;                                               
>>>                    \
>>> -    hook =
>>> __sec->addr;                                               
>>>                    \
>>> +    (hook) =
>>> __sec->addr;                                             
>>>                    \
>> 
>> This is not strictly needed, if not for consistency.
> 
> That explains why my first try didn't succeed.  But, why is it that 
> only
> MISRA only cares about bracketing for rvalues, not lvalues ?
> 

It was actually asked by Xen maintainers to introduce this special case 
on the grounds that it is extremely unlikely to result in an operator 
precedence mishap that leads to incorrect expansion. See below:

-doc_begin="Code violating Rule 20.7 is safe when macro parameters are 
used: (1)
as function arguments; (2) as macro arguments; (3) as array indices; (4) 
as lhs
in assignments; (5) as initializers, possibly designated, in initalizer 
lists;
(6) as the constant expression in a switch clause label."
-config=MC3A2.R20.7,expansion_context=
[...]


-- 
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253

Re: [PATCH 11/12] xen: Bracket uses of macro parameters
Posted by Andrew Cooper 5 days, 10 hours ago
On 25/02/2026 4:34 pm, Nicola Vetrini wrote:
> On 2026-02-25 17:05, Andrew Cooper wrote:
>> On 20/02/2026 10:45 pm, Nicola Vetrini wrote:
>>> On 2026-02-20 22:46, Andrew Cooper wrote:
>>>> Fixing Rule 20.7 violations.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>>
>> Thanks.
>>
>>>> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
>>>> index 7446533c8cfb..63473925cafb 100644
>>>> --- a/xen/common/livepatch.c
>>>> +++ b/xen/common/livepatch.c
>>>> @@ -664,9 +664,10 @@ static inline int
>>>> livepatch_check_expectations(const struct payload *payload)
>>>>      const struct livepatch_elf_sec *__sec =
>>>> livepatch_elf_sec_by_name(elf, section_name); \
>>>>      if ( !__sec
>>>> )                                                     
>>>>                    \
>>>>         
>>>> break;                                                        
>>>>                    \
>>>> -    if ( !section_ok(elf, __sec, sizeof(*hook)) ||
>>>> __sec->sec->sh_size != sizeof(*hook) ) \
>>>> +    if ( !section_ok(elf, __sec, sizeof(*(hook)))
>>>> ||                                      \
>>>> +         __sec->sec->sh_size != sizeof(*(hook))
>>>> )                                         \
>>>>          return
>>>> -EINVAL;                                               
>>>>                    \
>>>> -    hook =
>>>> __sec->addr;                                               
>>>>                    \
>>>> +    (hook) =
>>>> __sec->addr;                                             
>>>>                    \
>>>
>>> This is not strictly needed, if not for consistency.
>>
>> That explains why my first try didn't succeed.  But, why is it that only
>> MISRA only cares about bracketing for rvalues, not lvalues ?
>>
>
> It was actually asked by Xen maintainers to introduce this special
> case on the grounds that it is extremely unlikely to result in an
> operator precedence mishap that leads to incorrect expansion. See below:
>
> -doc_begin="Code violating Rule 20.7 is safe when macro parameters are
> used: (1)
> as function arguments; (2) as macro arguments; (3) as array indices;
> (4) as lhs
> in assignments; (5) as initializers, possibly designated, in
> initalizer lists;
> (6) as the constant expression in a switch clause label."
> -config=MC3A2.R20.7,expansion_context=
> [...] 

Oh ok.  I'll make it match what was agreed then.

Thanks,

~Andrew