[XEN PATCH 00/10] Address violations of MISRA C Rule 20.7

[XEN PATCH 00/10] Address violations of MISRA C Rule 20.7

[Nicola Vetrini]

Hi all,

this series aims to refactor some macros that cause violations of MISRA C Rule
20.7 ("Expressions resulting from the expansion of macro parameters shall be
enclosed in parentheses"). All the macros touched by these patches are in some
way involved in violations, and the strategy adopted to bring them into
compliance is to add parentheses around macro arguments where needed.

Nicola Vetrini (10):
  libelf: address violations of MISRA C Rule 20.7
  xen/page-defs: address violation of MISRA C Rule 20.7
  automation/eclair_analysis: deviate macro count_args_ for MISRA Rule
    20.7
  drivers: char: address violation of MISRA C Rule 20.7
  xen/spinlock: address violations of MISRA C Rule 20.7
  x86/pci: address violation of MISRA C Rule 20.7
  x86/acpi: power: address violations of MISRA Rule 20.7
  x86/hvm: hpet: address violations of MISRA C Rule 20.7
  x86/debugreg: address violation of MISRA C Rule 20.7
  x86/mm: address violations of MISRA C Rule 20.7

 automation/eclair_analysis/ECLAIR/deviations.ecl |  6 ++++++
 docs/misra/deviations.rst                        |  6 ++++++
 xen/arch/x86/acpi/power.c                        | 12 ++++++------
 xen/arch/x86/hvm/hpet.c                          |  4 ++--
 xen/arch/x86/include/asm/debugreg.h              |  2 +-
 xen/arch/x86/include/asm/pci.h                   |  8 ++++----
 xen/arch/x86/mm.c                                |  2 +-
 xen/common/libelf/libelf-private.h               |  2 +-
 xen/common/spinlock.c                            |  2 +-
 xen/drivers/char/omap-uart.c                     |  5 +++--
 xen/include/xen/libelf.h                         |  2 +-
 xen/include/xen/page-defs.h                      |  3 ++-
 xen/include/xen/spinlock.h                       |  2 +-
 13 files changed, 35 insertions(+), 21 deletions(-)

-- 
2.34.1

[XEN PATCH 01/10] libelf: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/common/libelf/libelf-private.h | 2 +-
 xen/include/xen/libelf.h           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/libelf/libelf-private.h b/xen/common/libelf/libelf-private.h
index 98cac65bc50d..197d7a7623a3 100644
--- a/xen/common/libelf/libelf-private.h
+++ b/xen/common/libelf/libelf-private.h
@@ -26,7 +26,7 @@
 /* we would like to use elf->log_callback but we can't because
  * there is no vprintk in Xen */
 #define elf_msg(elf, fmt, args ... ) \
-   if (elf->verbose) printk(fmt, ## args )
+   if ((elf)->verbose) printk(fmt, ## args )
 #define elf_err(elf, fmt, args ... ) \
    printk(fmt, ## args )
 
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 9ac530acc2a8..a0675a4dc352 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -288,7 +288,7 @@ bool elf_access_ok(struct elf_binary * elf,
 #define elf_store_val(elf, type, ptr, val)                              \
     ({                                                                  \
         typeof(type) elf_store__val = (val);                            \
-        elf_ptrval elf_store__targ = ptr;                               \
+        elf_ptrval elf_store__targ = (ptr);                             \
         if (elf_access_ok((elf), elf_store__targ,                       \
                           sizeof(elf_store__val))) {			\
             elf_memcpy_unchecked((void*)elf_store__targ, &elf_store__val, \
-- 
2.34.1

[XEN PATCH 02/10] xen/page-defs: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/include/xen/page-defs.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/include/xen/page-defs.h b/xen/include/xen/page-defs.h
index 540f8b0b6452..682da6b7b476 100644
--- a/xen/include/xen/page-defs.h
+++ b/xen/include/xen/page-defs.h
@@ -4,7 +4,8 @@
 /* Helpers for different page granularities. */
 #define PAGE_SIZE_GRAN(gran)        ((paddr_t)1 << PAGE_SHIFT_##gran)
 #define PAGE_MASK_GRAN(gran)        (-PAGE_SIZE_GRAN(gran))
-#define PAGE_ALIGN_GRAN(gran, addr) ((addr + ~PAGE_MASK_##gran) & PAGE_MASK_##gran)
+#define PAGE_ALIGN_GRAN(gran, addr) (((addr) + ~PAGE_MASK_##gran) & \
+                                     PAGE_MASK_##gran)
 
 #define PAGE_SHIFT_4K               12
 #define PAGE_SIZE_4K                PAGE_SIZE_GRAN(4K)
-- 
2.34.1

[XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Nicola Vetrini]

The count_args_ macro violates Rule 20.7, but it can't be made
compliant with Rule 20.7 without breaking its functionality. Since
it's very unlikely for this macro to be misused, it is deviated.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
 docs/misra/deviations.rst                        | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d21f112a9b94..c17e2f5a0886 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -445,6 +445,12 @@ not to parenthesize their arguments."
 -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^alternative_(v)?call[0-9]$))))"}
 -doc_end
 
+-doc_begin="The argument 'x' of the count_args_ macro can't be parenthesized as
+the rule would require, without breaking the functionality of the macro. The uses
+of this macro do not lead to developer confusion, and can thus be deviated."
+-config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"}
+-doc_end
+
 -doc_begin="Uses of variadic macros that have one of their arguments defined as
 a macro and used within the body for both ordinary parameter expansion and as an
 operand to the # or ## operators have a behavior that is well-understood and
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index ed0c1e8ed0bf..e228ae2e715f 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -371,6 +371,12 @@ Deviations related to MISRA C:2012 Rules:
        sanity checks in place.
      - Tagged as `safe` for ECLAIR.
 
+   * - R20.7
+     - The macro `count_args_` is not compliant with the rule, but is not likely
+       to incur in the risk of being misused or lead to developer confusion, and
+       refactoring it to add parentheses breaks its functionality.
+     - Tagged as `safe` for ECLAIR.
+
    * - R20.12
      - Variadic macros that use token pasting often employ the gcc extension
        `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, which is
-- 
2.34.1

[XEN PATCH 04/10] drivers: char: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional chage.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/drivers/char/omap-uart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
index 03b5b66e7acb..9e1abf306ace 100644
--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -48,8 +48,9 @@
 /* System configuration register */
 #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is enabled */
 
-#define omap_read(uart, off)       readl((uart)->regs + (off<<REG_SHIFT))
-#define omap_write(uart, off, val) writel((val), (uart)->regs + (off<<REG_SHIFT))
+#define omap_read(uart, off)       readl((uart)->regs + ((off) << REG_SHIFT))
+#define omap_write(uart, off, val) writel((val), (uart)->regs + \
+                                                 ((off) << REG_SHIFT))
 
 static struct omap_uart {
     u32 baud, clock_hz, data_bits, parity, stop_bits, fifo_size;
-- 
2.34.1

[XEN PATCH 05/10] xen/spinlock: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/common/spinlock.c      | 2 +-
 xen/include/xen/spinlock.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 5aa9ba618859..558ea7ac3518 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -269,7 +269,7 @@ void spin_debug_disable(void)
         profile->lock_cnt++;                                                 \
     }
 #define LOCK_PROFILE_VAR(var, val)    s_time_t var = (val)
-#define LOCK_PROFILE_BLOCK(var)       var = var ? : NOW()
+#define LOCK_PROFILE_BLOCK(var)       (var) = (var) ? : NOW()
 #define LOCK_PROFILE_BLKACC(tst, val)                                        \
     if ( tst )                                                               \
     {                                                                        \
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 18793c5e29cb..8825affb25ca 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -141,7 +141,7 @@ struct lock_profile_qhead {
         }                                                                     \
         prof->name = #l;                                                      \
         prof->ptr.lockptr = &(s)->l;                                          \
-        prof->is_rlock = isr;                                                 \
+        prof->is_rlock = (isr);                                               \
         prof->next = (s)->profile_head.elem_q;                                \
         (s)->profile_head.elem_q = prof;                                      \
     } while( 0 )
-- 
2.34.1

[XEN PATCH 06/10] x86/pci: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/include/asm/pci.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/pci.h b/xen/arch/x86/include/asm/pci.h
index 6bfe87e2780b..fd5480d67d43 100644
--- a/xen/arch/x86/include/asm/pci.h
+++ b/xen/arch/x86/include/asm/pci.h
@@ -8,10 +8,10 @@
 #define CF8_ADDR_HI(cf8) (  ((cf8) & 0x0f000000U) >> 16)
 #define CF8_ENABLED(cf8) (!!((cf8) & 0x80000000U))
 
-#define IS_SNB_GFX(id) (id == 0x01068086 || id == 0x01168086 \
-                        || id == 0x01268086 || id == 0x01028086 \
-                        || id == 0x01128086 || id == 0x01228086 \
-                        || id == 0x010A8086 )
+#define IS_SNB_GFX(id) ((id) == 0x01068086 || (id) == 0x01168086 \
+                        || (id) == 0x01268086 || (id) == 0x01028086 \
+                        || (id) == 0x01128086 || (id) == 0x01228086 \
+                        || (id) == 0x010A8086 )
 
 struct arch_pci_dev {
     vmask_t used_vectors;
-- 
2.34.1

[XEN PATCH 07/10] x86/acpi: power: address violations of MISRA Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/acpi/power.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index c6fa810a6b13..610937f42e95 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -422,12 +422,12 @@ static void tboot_sleep(u8 sleep_state)
 {
     uint32_t shutdown_type;
 
-#define TB_COPY_GAS(tbg, g)             \
-    tbg.space_id = g.space_id;          \
-    tbg.bit_width = g.bit_width;        \
-    tbg.bit_offset = g.bit_offset;      \
-    tbg.access_width = g.access_width;  \
-    tbg.address = g.address;
+#define TB_COPY_GAS(tbg, g)                 \
+    (tbg).space_id = (g).space_id;          \
+    (tbg).bit_width = (g).bit_width;        \
+    (tbg).bit_offset = (g).bit_offset;      \
+    (tbg).access_width = (g).access_width;  \
+    (tbg).address = (g).address;
 
     /* sizes are not same (due to packing) so copy each one */
     TB_COPY_GAS(g_tboot_shared->acpi_sinfo.pm1a_cnt_blk,
-- 
2.34.1

[XEN PATCH 08/10] x86/hvm: hpet: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/hvm/hpet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index 1db9c0b60ee0..5f456221cbfb 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -43,11 +43,11 @@
     ((s_time_t)((((tick) > (h)->hpet_to_ns_limit) ?     \
         ~0ULL : (tick) * (h)->hpet_to_ns_scale) >> 10))
 
-#define timer_config(h, n)       (h->hpet.timers[n].config)
+#define timer_config(h, n)       ((h)->hpet.timers[n].config)
 #define timer_enabled(h, n)      (timer_config(h, n) & HPET_TN_ENABLE)
 #define timer_is_periodic(h, n)  (timer_config(h, n) & HPET_TN_PERIODIC)
 #define timer_is_32bit(h, n)     (timer_config(h, n) & HPET_TN_32BIT)
-#define hpet_enabled(h)          (h->hpet.config & HPET_CFG_ENABLE)
+#define hpet_enabled(h)          ((h)->hpet.config & HPET_CFG_ENABLE)
 #define timer_level(h, n)        (timer_config(h, n) & HPET_TN_LEVEL)
 
 #define timer_int_route(h, n)    MASK_EXTR(timer_config(h, n), HPET_TN_ROUTE)
-- 
2.34.1

[XEN PATCH 09/10] x86/debugreg: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/include/asm/debugreg.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/debugreg.h b/xen/arch/x86/include/asm/debugreg.h
index 2bdaf5d9aa11..96c406ad53c8 100644
--- a/xen/arch/x86/include/asm/debugreg.h
+++ b/xen/arch/x86/include/asm/debugreg.h
@@ -67,7 +67,7 @@
 #define DR_GENERAL_DETECT        (0x00002000UL) /* General detect enable */
 
 #define write_debugreg(reg, val) do {                       \
-    unsigned long __val = val;                              \
+    unsigned long __val = (val);                            \
     asm volatile ( "mov %0,%%db" #reg : : "r" (__val) );    \
 } while (0)
 #define read_debugreg(reg) ({                               \
-- 
2.34.1

[XEN PATCH 10/10] x86/mm: address violations of MISRA C Rule 20.7

[Nicola Vetrini]

MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses". Therefore, some
macro definitions should gain additional parentheses to ensure that all
current and future users will be safe with respect to expansions that
can possibly alter the semantics of the passed-in macro parameter.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/x86/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 9141912ae52d..87529db7d136 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -162,7 +162,7 @@ static uint32_t base_disallow_mask;
 #define L4_DISALLOW_MASK (base_disallow_mask)
 
 #define l1_disallow_mask(d)                                     \
-    ((d != dom_io) &&                                           \
+    (((d) != dom_io) &&                                         \
      (rangeset_is_empty((d)->iomem_caps) &&                     \
       rangeset_is_empty((d)->arch.ioport_caps) &&               \
       !has_arch_pdevs(d) &&                                     \
-- 
2.34.1

Re: [XEN PATCH 01/10] libelf: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 02/10] xen/page-defs: address violation of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 04/10] drivers: char: address violation of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> --- a/xen/drivers/char/omap-uart.c
> +++ b/xen/drivers/char/omap-uart.c
> @@ -48,8 +48,9 @@
>  /* System configuration register */
>  #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is enabled */
>  
> -#define omap_read(uart, off)       readl((uart)->regs + (off<<REG_SHIFT))
> -#define omap_write(uart, off, val) writel((val), (uart)->regs + (off<<REG_SHIFT))
> +#define omap_read(uart, off)       readl((uart)->regs + ((off) << REG_SHIFT))
> +#define omap_write(uart, off, val) writel((val), (uart)->regs + \

Would have been nice to drop the excess parentheses at the same time.

Jan

Re: [XEN PATCH 05/10] xen/spinlock: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 06/10] x86/pci: address violation of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 07/10] x86/acpi: power: address violations of MISRA Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 08/10] x86/hvm: hpet: address violations of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 09/10] x86/debugreg: address violation of MISRA C Rule 20.7

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

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

[Jan Beulich]

On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

Re: [XEN PATCH 04/10] drivers: char: address violation of MISRA C Rule 20.7

[Nicola Vetrini]

On 2024-04-24 09:23, Jan Beulich wrote:
> On 23.04.2024 17:12, Nicola Vetrini wrote:
>> --- a/xen/drivers/char/omap-uart.c
>> +++ b/xen/drivers/char/omap-uart.c
>> @@ -48,8 +48,9 @@
>>  /* System configuration register */
>>  #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is 
>> enabled */
>> 
>> -#define omap_read(uart, off)       readl((uart)->regs + 
>> (off<<REG_SHIFT))
>> -#define omap_write(uart, off, val) writel((val), (uart)->regs + 
>> (off<<REG_SHIFT))
>> +#define omap_read(uart, off)       readl((uart)->regs + ((off) << 
>> REG_SHIFT))
>> +#define omap_write(uart, off, val) writel((val), (uart)->regs + \
> 
> Would have been nice to drop the excess parentheses at the same time.
> 
> Jan

Right. I think I'll have a few more patches on this rule, so maybe I can 
adjust it.

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

Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Stefano Stabellini]

On Tue, 23 Apr 2024, Nicola Vetrini wrote:
> The count_args_ macro violates Rule 20.7, but it can't be made
> compliant with Rule 20.7 without breaking its functionality. Since
> it's very unlikely for this macro to be misused, it is deviated.

That is OK but can't we use the SAF- framework to do it, given that it
is just one macro?

If not, this is also OK.


> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
>  docs/misra/deviations.rst                        | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index d21f112a9b94..c17e2f5a0886 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -445,6 +445,12 @@ not to parenthesize their arguments."
>  -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^alternative_(v)?call[0-9]$))))"}
>  -doc_end
>  
> +-doc_begin="The argument 'x' of the count_args_ macro can't be parenthesized as
> +the rule would require, without breaking the functionality of the macro. The uses
> +of this macro do not lead to developer confusion, and can thus be deviated."
> +-config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"}
> +-doc_end
> +
>  -doc_begin="Uses of variadic macros that have one of their arguments defined as
>  a macro and used within the body for both ordinary parameter expansion and as an
>  operand to the # or ## operators have a behavior that is well-understood and
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index ed0c1e8ed0bf..e228ae2e715f 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -371,6 +371,12 @@ Deviations related to MISRA C:2012 Rules:
>         sanity checks in place.
>       - Tagged as `safe` for ECLAIR.
>  
> +   * - R20.7
> +     - The macro `count_args_` is not compliant with the rule, but is not likely
> +       to incur in the risk of being misused or lead to developer confusion, and
> +       refactoring it to add parentheses breaks its functionality.
> +     - Tagged as `safe` for ECLAIR.
> +
>     * - R20.12
>       - Variadic macros that use token pasting often employ the gcc extension
>         `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, which is
> -- 
> 2.34.1
> 

Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Nicola Vetrini]

On 2024-04-25 02:28, Stefano Stabellini wrote:
> On Tue, 23 Apr 2024, Nicola Vetrini wrote:
>> The count_args_ macro violates Rule 20.7, but it can't be made
>> compliant with Rule 20.7 without breaking its functionality. Since
>> it's very unlikely for this macro to be misused, it is deviated.
> 
> That is OK but can't we use the SAF- framework to do it, given that it
> is just one macro?
> 
> If not, this is also OK.
> 
> 

It would be more fragile, for no substantial gain

>> No functional change.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>>  automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
>>  docs/misra/deviations.rst                        | 6 ++++++
>>  2 files changed, 12 insertions(+)
>> 
>> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
>> b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> index d21f112a9b94..c17e2f5a0886 100644
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -445,6 +445,12 @@ not to parenthesize their arguments."
>>  -config=MC3R1.R20.7,reports+={safe, 
>> "any_area(any_loc(any_exp(macro(^alternative_(v)?call[0-9]$))))"}
>>  -doc_end
>> 
>> +-doc_begin="The argument 'x' of the count_args_ macro can't be 
>> parenthesized as
>> +the rule would require, without breaking the functionality of the 
>> macro. The uses
>> +of this macro do not lead to developer confusion, and can thus be 
>> deviated."
>> +-config=MC3R1.R20.7,reports+={safe, 
>> "any_area(any_loc(any_exp(macro(^count_args_$))))"}
>> +-doc_end
>> +
>>  -doc_begin="Uses of variadic macros that have one of their arguments 
>> defined as
>>  a macro and used within the body for both ordinary parameter 
>> expansion and as an
>>  operand to the # or ## operators have a behavior that is 
>> well-understood and
>> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
>> index ed0c1e8ed0bf..e228ae2e715f 100644
>> --- a/docs/misra/deviations.rst
>> +++ b/docs/misra/deviations.rst
>> @@ -371,6 +371,12 @@ Deviations related to MISRA C:2012 Rules:
>>         sanity checks in place.
>>       - Tagged as `safe` for ECLAIR.
>> 
>> +   * - R20.7
>> +     - The macro `count_args_` is not compliant with the rule, but is 
>> not likely
>> +       to incur in the risk of being misused or lead to developer 
>> confusion, and
>> +       refactoring it to add parentheses breaks its functionality.
>> +     - Tagged as `safe` for ECLAIR.
>> +
>>     * - R20.12
>>       - Variadic macros that use token pasting often employ the gcc 
>> extension
>>         `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, 
>> which is
>> --
>> 2.34.1
>> 

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

Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Stefano Stabellini]

On Mon, 29 Apr 2024, Nicola Vetrini wrote:
> On 2024-04-25 02:28, Stefano Stabellini wrote:
> > On Tue, 23 Apr 2024, Nicola Vetrini wrote:
> > > The count_args_ macro violates Rule 20.7, but it can't be made
> > > compliant with Rule 20.7 without breaking its functionality. Since
> > > it's very unlikely for this macro to be misused, it is deviated.
> > 
> > That is OK but can't we use the SAF- framework to do it, given that it
> > is just one macro?
> > 
> > If not, this is also OK.
> > 
> > 
> 
> It would be more fragile, for no substantial gain

OK

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Nicola Vetrini]

On 2024-05-01 21:54, Stefano Stabellini wrote:
> On Mon, 29 Apr 2024, Nicola Vetrini wrote:
>> On 2024-04-25 02:28, Stefano Stabellini wrote:
>> > On Tue, 23 Apr 2024, Nicola Vetrini wrote:
>> > > The count_args_ macro violates Rule 20.7, but it can't be made
>> > > compliant with Rule 20.7 without breaking its functionality. Since
>> > > it's very unlikely for this macro to be misused, it is deviated.
>> >
>> > That is OK but can't we use the SAF- framework to do it, given that it
>> > is just one macro?
>> >
>> > If not, this is also OK.
>> >
>> >
>> 
>> It would be more fragile, for no substantial gain
> 
> OK
> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

Hi,

I think this patch slipped through the cracks. I see it only has 
Stefano's R-by, so perhaps it needs a further ack?

Thanks,

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

Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

[Jan Beulich]

On 15.05.2024 09:09, Nicola Vetrini wrote:
> On 2024-05-01 21:54, Stefano Stabellini wrote:
>> On Mon, 29 Apr 2024, Nicola Vetrini wrote:
>>> On 2024-04-25 02:28, Stefano Stabellini wrote:
>>>> On Tue, 23 Apr 2024, Nicola Vetrini wrote:
>>>>> The count_args_ macro violates Rule 20.7, but it can't be made
>>>>> compliant with Rule 20.7 without breaking its functionality. Since
>>>>> it's very unlikely for this macro to be misused, it is deviated.
>>>>
>>>> That is OK but can't we use the SAF- framework to do it, given that it
>>>> is just one macro?
>>>>
>>>> If not, this is also OK.
>>>>
>>>>
>>>
>>> It would be more fragile, for no substantial gain
>>
>> OK
>>
>> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> I think this patch slipped through the cracks. I see it only has 
> Stefano's R-by, so perhaps it needs a further ack?

I don't think it does. All it needs is putting in. I guess I assumed
Stefano would be taking care of that, with your discussion (quickly)
settled.

Jan