| Message ID | a4a195005742ef7833da354859784c4506692821.1709896401.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address some violations of MISRA C Rule 20.7 | expand |
On Fri, 8 Mar 2024, 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> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> There are a couple of cases below where parentheses are not strictly necessary but this patch is following the decided deviation pattern. > --- > Style in arm64/cpufeature.c has not been amended, because this file seems > to be kept in sync with its Linux counterpart. > > Changes in v2: > - Added parentheses for consistency > --- > xen/arch/arm/arm64/cpufeature.c | 14 ++++----- > xen/arch/arm/cpuerrata.c | 8 +++--- > xen/arch/arm/include/asm/arm64/sysregs.h | 2 +- > xen/arch/arm/include/asm/guest_atomics.h | 4 +-- > xen/arch/arm/include/asm/mm.h | 2 +- > xen/arch/arm/include/asm/smccc.h | 36 ++++++++++++------------ > xen/arch/arm/include/asm/vgic-emul.h | 8 +++--- > xen/arch/arm/vcpreg.c | 5 ++-- > 8 files changed, 40 insertions(+), 39 deletions(-) > > diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c > index 864413d9cc03..6fb8974ade7f 100644 > --- a/xen/arch/arm/arm64/cpufeature.c > +++ b/xen/arch/arm/arm64/cpufeature.c > @@ -78,13 +78,13 @@ > > #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ > { \ > - .sign = SIGNED, \ > - .visible = VISIBLE, \ > - .strict = STRICT, \ > - .type = TYPE, \ > - .shift = SHIFT, \ > - .width = WIDTH, \ > - .safe_val = SAFE_VAL, \ > + .sign = (SIGNED), \ > + .visible = (VISIBLE), \ > + .strict = (STRICT), \ > + .type = (TYPE), \ > + .shift = (SHIFT), \ > + .width = (WIDTH), \ > + .safe_val = (SAFE_VAL), \ > } > > /* Define a feature with unsigned values */ > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index a28fa6ac78cc..2b7101ea2524 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -461,13 +461,13 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > > #define MIDR_RANGE(model, min, max) \ > .matches = is_affected_midr_range, \ > - .midr_model = model, \ > - .midr_range_min = min, \ > - .midr_range_max = max > + .midr_model = (model), \ > + .midr_range_min = (min), \ > + .midr_range_max = (max) > > #define MIDR_ALL_VERSIONS(model) \ > .matches = is_affected_midr_range, \ > - .midr_model = model, \ > + .midr_model = (model), \ > .midr_range_min = 0, \ > .midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK) > > diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h > index 3fdeb9d8cdef..b593e4028b53 100644 > --- a/xen/arch/arm/include/asm/arm64/sysregs.h > +++ b/xen/arch/arm/include/asm/arm64/sysregs.h > @@ -465,7 +465,7 @@ > /* Access to system registers */ > > #define WRITE_SYSREG64(v, name) do { \ > - uint64_t _r = v; \ > + uint64_t _r = (v); \ > asm volatile("msr "__stringify(name)", %0" : : "r" (_r)); \ > } while (0) > #define READ_SYSREG64(name) ({ \ > diff --git a/xen/arch/arm/include/asm/guest_atomics.h b/xen/arch/arm/include/asm/guest_atomics.h > index a1745f8613f6..8893eb9a55d7 100644 > --- a/xen/arch/arm/include/asm/guest_atomics.h > +++ b/xen/arch/arm/include/asm/guest_atomics.h > @@ -32,7 +32,7 @@ static inline void guest_##name(struct domain *d, int nr, volatile void *p) \ > perfc_incr(atomics_guest_paused); \ > \ > domain_pause_nosync(d); \ > - name(nr, p); \ > + (name)(nr, p); \ > domain_unpause(d); \ > } > > @@ -52,7 +52,7 @@ static inline int guest_##name(struct domain *d, int nr, volatile void *p) \ > perfc_incr(atomics_guest_paused); \ > \ > domain_pause_nosync(d); \ > - oldbit = name(nr, p); \ > + oldbit = (name)(nr, p); \ > domain_unpause(d); \ > \ > return oldbit; \ > diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h > index cbcf3bf14767..48538b5337aa 100644 > --- a/xen/arch/arm/include/asm/mm.h > +++ b/xen/arch/arm/include/asm/mm.h > @@ -250,7 +250,7 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len) > #define gaddr_to_gfn(ga) _gfn(paddr_to_pfn(ga)) > #define mfn_to_maddr(mfn) pfn_to_paddr(mfn_x(mfn)) > #define maddr_to_mfn(ma) _mfn(paddr_to_pfn(ma)) > -#define vmap_to_mfn(va) maddr_to_mfn(virt_to_maddr((vaddr_t)va)) > +#define vmap_to_mfn(va) maddr_to_mfn(virt_to_maddr((vaddr_t)(va))) > #define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va)) > > /* Page-align address and convert to frame number format */ > diff --git a/xen/arch/arm/include/asm/smccc.h b/xen/arch/arm/include/asm/smccc.h > index 1adcd37443c7..a289c48b7ffd 100644 > --- a/xen/arch/arm/include/asm/smccc.h > +++ b/xen/arch/arm/include/asm/smccc.h > @@ -122,56 +122,56 @@ struct arm_smccc_res { > #define __constraint_read_7 __constraint_read_6, "r" (r7) > > #define __declare_arg_0(a0, res) \ > - struct arm_smccc_res *___res = res; \ > - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ > + struct arm_smccc_res *___res = (res); \ > + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ > register unsigned long r1 ASM_REG(1); \ > register unsigned long r2 ASM_REG(2); \ > register unsigned long r3 ASM_REG(3) > > #define __declare_arg_1(a0, a1, res) \ > - typeof(a1) __a1 = a1; \ > - struct arm_smccc_res *___res = res; \ > - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ > + typeof(a1) __a1 = (a1); \ > + struct arm_smccc_res *___res = (res); \ > + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ > register unsigned long r1 ASM_REG(1) = __a1; \ > register unsigned long r2 ASM_REG(2); \ > register unsigned long r3 ASM_REG(3) > > #define __declare_arg_2(a0, a1, a2, res) \ > - typeof(a1) __a1 = a1; \ > - typeof(a2) __a2 = a2; \ > - struct arm_smccc_res *___res = res; \ > - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ > + typeof(a1) __a1 = (a1); \ > + typeof(a2) __a2 = (a2); \ > + struct arm_smccc_res *___res = (res); \ > + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ > register unsigned long r1 ASM_REG(1) = __a1; \ > register unsigned long r2 ASM_REG(2) = __a2; \ > register unsigned long r3 ASM_REG(3) > > #define __declare_arg_3(a0, a1, a2, a3, res) \ > - typeof(a1) __a1 = a1; \ > - typeof(a2) __a2 = a2; \ > - typeof(a3) __a3 = a3; \ > - struct arm_smccc_res *___res = res; \ > - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ > + typeof(a1) __a1 = (a1); \ > + typeof(a2) __a2 = (a2); \ > + typeof(a3) __a3 = (a3); \ > + struct arm_smccc_res *___res = (res); \ > + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ > register unsigned long r1 ASM_REG(1) = __a1; \ > register unsigned long r2 ASM_REG(2) = __a2; \ > register unsigned long r3 ASM_REG(3) = __a3 > > #define __declare_arg_4(a0, a1, a2, a3, a4, res) \ > - typeof(a4) __a4 = a4; \ > + typeof(a4) __a4 = (a4); \ > __declare_arg_3(a0, a1, a2, a3, res); \ > register unsigned long r4 ASM_REG(4) = __a4 > > #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res) \ > - typeof(a5) __a5 = a5; \ > + typeof(a5) __a5 = (a5); \ > __declare_arg_4(a0, a1, a2, a3, a4, res); \ > register typeof(a5) r5 ASM_REG(5) = __a5 > > #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res) \ > - typeof(a6) __a6 = a6; \ > + typeof(a6) __a6 = (a6); \ > __declare_arg_5(a0, a1, a2, a3, a4, a5, res); \ > register typeof(a6) r6 ASM_REG(6) = __a6 > > #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res) \ > - typeof(a7) __a7 = a7; \ > + typeof(a7) __a7 = (a7); \ > __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res); \ > register typeof(a7) r7 ASM_REG(7) = __a7 > > diff --git a/xen/arch/arm/include/asm/vgic-emul.h b/xen/arch/arm/include/asm/vgic-emul.h > index e52fbaa3ec04..fd0cfa2175fe 100644 > --- a/xen/arch/arm/include/asm/vgic-emul.h > +++ b/xen/arch/arm/include/asm/vgic-emul.h > @@ -6,11 +6,11 @@ > * a range of registers > */ > > -#define VREG32(reg) reg ... reg + 3 > -#define VREG64(reg) reg ... reg + 7 > +#define VREG32(reg) (reg) ... ((reg) + 3) > +#define VREG64(reg) (reg) ... ((reg) + 7) > > -#define VRANGE32(start, end) start ... end + 3 > -#define VRANGE64(start, end) start ... end + 7 > +#define VRANGE32(start, end) (start) ... ((end) + 3) > +#define VRANGE64(start, end) (start) ... ((end) + 7) > > /* > * 64 bits registers can be accessible using 32-bit and 64-bit unless > diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c > index a2d050070473..019cf34f003a 100644 > --- a/xen/arch/arm/vcpreg.c > +++ b/xen/arch/arm/vcpreg.c > @@ -39,7 +39,8 @@ > */ > > #ifdef CONFIG_ARM_64 > -#define WRITE_SYSREG_SZ(sz, val, sysreg) WRITE_SYSREG((uint##sz##_t)val, sysreg) > +#define WRITE_SYSREG_SZ(sz, val, sysreg) \ > + WRITE_SYSREG((uint##sz##_t)(val), sysreg) > #else > /* > * WRITE_SYSREG{32/64} on arm32 is defined as variadic macro which imposes > @@ -64,7 +65,7 @@ static bool func(struct cpu_user_regs *regs, type##sz##_t *r, bool read) \ > bool cache_enabled = vcpu_has_cache_enabled(v); \ > \ > GUEST_BUG_ON(read); \ > - WRITE_SYSREG_SZ(sz, *r, reg); \ > + WRITE_SYSREG_SZ(sz, *(r), reg); \ > \ > p2m_toggle_cache(v, cache_enabled); \ > \ > -- > 2.34.1 >
diff --git a/xen/arch/arm/arm64/cpufeature.c b/xen/arch/arm/arm64/cpufeature.c index 864413d9cc03..6fb8974ade7f 100644 --- a/xen/arch/arm/arm64/cpufeature.c +++ b/xen/arch/arm/arm64/cpufeature.c @@ -78,13 +78,13 @@ #define __ARM64_FTR_BITS(SIGNED, VISIBLE, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \ { \ - .sign = SIGNED, \ - .visible = VISIBLE, \ - .strict = STRICT, \ - .type = TYPE, \ - .shift = SHIFT, \ - .width = WIDTH, \ - .safe_val = SAFE_VAL, \ + .sign = (SIGNED), \ + .visible = (VISIBLE), \ + .strict = (STRICT), \ + .type = (TYPE), \ + .shift = (SHIFT), \ + .width = (WIDTH), \ + .safe_val = (SAFE_VAL), \ } /* Define a feature with unsigned values */ diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index a28fa6ac78cc..2b7101ea2524 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -461,13 +461,13 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) #define MIDR_RANGE(model, min, max) \ .matches = is_affected_midr_range, \ - .midr_model = model, \ - .midr_range_min = min, \ - .midr_range_max = max + .midr_model = (model), \ + .midr_range_min = (min), \ + .midr_range_max = (max) #define MIDR_ALL_VERSIONS(model) \ .matches = is_affected_midr_range, \ - .midr_model = model, \ + .midr_model = (model), \ .midr_range_min = 0, \ .midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK) diff --git a/xen/arch/arm/include/asm/arm64/sysregs.h b/xen/arch/arm/include/asm/arm64/sysregs.h index 3fdeb9d8cdef..b593e4028b53 100644 --- a/xen/arch/arm/include/asm/arm64/sysregs.h +++ b/xen/arch/arm/include/asm/arm64/sysregs.h @@ -465,7 +465,7 @@ /* Access to system registers */ #define WRITE_SYSREG64(v, name) do { \ - uint64_t _r = v; \ + uint64_t _r = (v); \ asm volatile("msr "__stringify(name)", %0" : : "r" (_r)); \ } while (0) #define READ_SYSREG64(name) ({ \ diff --git a/xen/arch/arm/include/asm/guest_atomics.h b/xen/arch/arm/include/asm/guest_atomics.h index a1745f8613f6..8893eb9a55d7 100644 --- a/xen/arch/arm/include/asm/guest_atomics.h +++ b/xen/arch/arm/include/asm/guest_atomics.h @@ -32,7 +32,7 @@ static inline void guest_##name(struct domain *d, int nr, volatile void *p) \ perfc_incr(atomics_guest_paused); \ \ domain_pause_nosync(d); \ - name(nr, p); \ + (name)(nr, p); \ domain_unpause(d); \ } @@ -52,7 +52,7 @@ static inline int guest_##name(struct domain *d, int nr, volatile void *p) \ perfc_incr(atomics_guest_paused); \ \ domain_pause_nosync(d); \ - oldbit = name(nr, p); \ + oldbit = (name)(nr, p); \ domain_unpause(d); \ \ return oldbit; \ diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h index cbcf3bf14767..48538b5337aa 100644 --- a/xen/arch/arm/include/asm/mm.h +++ b/xen/arch/arm/include/asm/mm.h @@ -250,7 +250,7 @@ static inline void __iomem *ioremap_wc(paddr_t start, size_t len) #define gaddr_to_gfn(ga) _gfn(paddr_to_pfn(ga)) #define mfn_to_maddr(mfn) pfn_to_paddr(mfn_x(mfn)) #define maddr_to_mfn(ma) _mfn(paddr_to_pfn(ma)) -#define vmap_to_mfn(va) maddr_to_mfn(virt_to_maddr((vaddr_t)va)) +#define vmap_to_mfn(va) maddr_to_mfn(virt_to_maddr((vaddr_t)(va))) #define vmap_to_page(va) mfn_to_page(vmap_to_mfn(va)) /* Page-align address and convert to frame number format */ diff --git a/xen/arch/arm/include/asm/smccc.h b/xen/arch/arm/include/asm/smccc.h index 1adcd37443c7..a289c48b7ffd 100644 --- a/xen/arch/arm/include/asm/smccc.h +++ b/xen/arch/arm/include/asm/smccc.h @@ -122,56 +122,56 @@ struct arm_smccc_res { #define __constraint_read_7 __constraint_read_6, "r" (r7) #define __declare_arg_0(a0, res) \ - struct arm_smccc_res *___res = res; \ - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ + struct arm_smccc_res *___res = (res); \ + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ register unsigned long r1 ASM_REG(1); \ register unsigned long r2 ASM_REG(2); \ register unsigned long r3 ASM_REG(3) #define __declare_arg_1(a0, a1, res) \ - typeof(a1) __a1 = a1; \ - struct arm_smccc_res *___res = res; \ - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ + typeof(a1) __a1 = (a1); \ + struct arm_smccc_res *___res = (res); \ + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ register unsigned long r1 ASM_REG(1) = __a1; \ register unsigned long r2 ASM_REG(2); \ register unsigned long r3 ASM_REG(3) #define __declare_arg_2(a0, a1, a2, res) \ - typeof(a1) __a1 = a1; \ - typeof(a2) __a2 = a2; \ - struct arm_smccc_res *___res = res; \ - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ + typeof(a1) __a1 = (a1); \ + typeof(a2) __a2 = (a2); \ + struct arm_smccc_res *___res = (res); \ + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ register unsigned long r1 ASM_REG(1) = __a1; \ register unsigned long r2 ASM_REG(2) = __a2; \ register unsigned long r3 ASM_REG(3) #define __declare_arg_3(a0, a1, a2, a3, res) \ - typeof(a1) __a1 = a1; \ - typeof(a2) __a2 = a2; \ - typeof(a3) __a3 = a3; \ - struct arm_smccc_res *___res = res; \ - register unsigned long r0 ASM_REG(0) = (uint32_t)a0; \ + typeof(a1) __a1 = (a1); \ + typeof(a2) __a2 = (a2); \ + typeof(a3) __a3 = (a3); \ + struct arm_smccc_res *___res = (res); \ + register unsigned long r0 ASM_REG(0) = (uint32_t)(a0); \ register unsigned long r1 ASM_REG(1) = __a1; \ register unsigned long r2 ASM_REG(2) = __a2; \ register unsigned long r3 ASM_REG(3) = __a3 #define __declare_arg_4(a0, a1, a2, a3, a4, res) \ - typeof(a4) __a4 = a4; \ + typeof(a4) __a4 = (a4); \ __declare_arg_3(a0, a1, a2, a3, res); \ register unsigned long r4 ASM_REG(4) = __a4 #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res) \ - typeof(a5) __a5 = a5; \ + typeof(a5) __a5 = (a5); \ __declare_arg_4(a0, a1, a2, a3, a4, res); \ register typeof(a5) r5 ASM_REG(5) = __a5 #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res) \ - typeof(a6) __a6 = a6; \ + typeof(a6) __a6 = (a6); \ __declare_arg_5(a0, a1, a2, a3, a4, a5, res); \ register typeof(a6) r6 ASM_REG(6) = __a6 #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res) \ - typeof(a7) __a7 = a7; \ + typeof(a7) __a7 = (a7); \ __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res); \ register typeof(a7) r7 ASM_REG(7) = __a7 diff --git a/xen/arch/arm/include/asm/vgic-emul.h b/xen/arch/arm/include/asm/vgic-emul.h index e52fbaa3ec04..fd0cfa2175fe 100644 --- a/xen/arch/arm/include/asm/vgic-emul.h +++ b/xen/arch/arm/include/asm/vgic-emul.h @@ -6,11 +6,11 @@ * a range of registers */ -#define VREG32(reg) reg ... reg + 3 -#define VREG64(reg) reg ... reg + 7 +#define VREG32(reg) (reg) ... ((reg) + 3) +#define VREG64(reg) (reg) ... ((reg) + 7) -#define VRANGE32(start, end) start ... end + 3 -#define VRANGE64(start, end) start ... end + 7 +#define VRANGE32(start, end) (start) ... ((end) + 3) +#define VRANGE64(start, end) (start) ... ((end) + 7) /* * 64 bits registers can be accessible using 32-bit and 64-bit unless diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c index a2d050070473..019cf34f003a 100644 --- a/xen/arch/arm/vcpreg.c +++ b/xen/arch/arm/vcpreg.c @@ -39,7 +39,8 @@ */ #ifdef CONFIG_ARM_64 -#define WRITE_SYSREG_SZ(sz, val, sysreg) WRITE_SYSREG((uint##sz##_t)val, sysreg) +#define WRITE_SYSREG_SZ(sz, val, sysreg) \ + WRITE_SYSREG((uint##sz##_t)(val), sysreg) #else /* * WRITE_SYSREG{32/64} on arm32 is defined as variadic macro which imposes @@ -64,7 +65,7 @@ static bool func(struct cpu_user_regs *regs, type##sz##_t *r, bool read) \ bool cache_enabled = vcpu_has_cache_enabled(v); \ \ GUEST_BUG_ON(read); \ - WRITE_SYSREG_SZ(sz, *r, reg); \ + WRITE_SYSREG_SZ(sz, *(r), reg); \ \ p2m_toggle_cache(v, cache_enabled); \ \
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> --- Style in arm64/cpufeature.c has not been amended, because this file seems to be kept in sync with its Linux counterpart. Changes in v2: - Added parentheses for consistency --- xen/arch/arm/arm64/cpufeature.c | 14 ++++----- xen/arch/arm/cpuerrata.c | 8 +++--- xen/arch/arm/include/asm/arm64/sysregs.h | 2 +- xen/arch/arm/include/asm/guest_atomics.h | 4 +-- xen/arch/arm/include/asm/mm.h | 2 +- xen/arch/arm/include/asm/smccc.h | 36 ++++++++++++------------ xen/arch/arm/include/asm/vgic-emul.h | 8 +++--- xen/arch/arm/vcpreg.c | 5 ++-- 8 files changed, 40 insertions(+), 39 deletions(-)