| Message ID | 6d14b3283005cf1a30c4fa24f9841586a41e2b1b.1715757982.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address violations of MISRA C Rule 20.7 | expand |
On Wed, 15 May 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> > --- > xen/arch/x86/hvm/mtrr.c | 2 +- > xen/arch/x86/hvm/rtc.c | 2 +- > xen/arch/x86/include/asm/hvm/save.h | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c > index 32f74c1db03b..1079851f70ed 100644 > --- a/xen/arch/x86/hvm/mtrr.c > +++ b/xen/arch/x86/hvm/mtrr.c > @@ -16,7 +16,7 @@ > #include <public/hvm/e820.h> > > /* Get page attribute fields (PAn) from PAT MSR. */ > -#define pat_cr_2_paf(pat_cr,n) ((((uint64_t)pat_cr) >> ((n)<<3)) & 0xff) > +#define pat_cr_2_paf(pat_cr, n) ((((uint64_t)(pat_cr)) >> ((n) << 3)) & 0xff) just a cosmetic change > /* Effective mm type lookup table, according to MTRR and PAT. */ > static const uint8_t mm_type_tbl[MTRR_NUM_TYPES][X86_NUM_MT] = { > diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c > index 4bb1c7505534..72c7bdbfcd02 100644 > --- a/xen/arch/x86/hvm/rtc.c > +++ b/xen/arch/x86/hvm/rtc.c > @@ -45,7 +45,7 @@ > #define vrtc_domain(x) (container_of(x, struct pl_time, vrtc)->domain) > #define vrtc_vcpu(x) (pt_global_vcpu_target(vrtc_domain(x))) > #define epoch_year 1900 > -#define get_year(x) (x + epoch_year) > +#define get_year(x) ((x) + epoch_year) > > enum rtc_mode { > rtc_mode_no_ack, > diff --git a/xen/arch/x86/include/asm/hvm/save.h b/xen/arch/x86/include/asm/hvm/save.h > index 8149aa113cb4..ec8de029319d 100644 > --- a/xen/arch/x86/include/asm/hvm/save.h > +++ b/xen/arch/x86/include/asm/hvm/save.h > @@ -50,7 +50,7 @@ int _hvm_check_entry(struct hvm_domain_context *h, > HVM_SAVE_LENGTH(x), true) == 0 ) \ > { \ > ptr = &(h)->data[(h)->cur]; \ > - h->cur += HVM_SAVE_LENGTH(x); \ > + (h)->cur += HVM_SAVE_LENGTH(x); \ > } \ > ptr; }) > > -- > 2.34.1 >
On 2024-05-16 01:18, Stefano Stabellini wrote: > On Wed, 15 May 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> > > >> --- >> xen/arch/x86/hvm/mtrr.c | 2 +- >> xen/arch/x86/hvm/rtc.c | 2 +- >> xen/arch/x86/include/asm/hvm/save.h | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c >> index 32f74c1db03b..1079851f70ed 100644 >> --- a/xen/arch/x86/hvm/mtrr.c >> +++ b/xen/arch/x86/hvm/mtrr.c >> @@ -16,7 +16,7 @@ >> #include <public/hvm/e820.h> >> >> /* Get page attribute fields (PAn) from PAT MSR. */ >> -#define pat_cr_2_paf(pat_cr,n) ((((uint64_t)pat_cr) >> ((n)<<3)) & >> 0xff) >> +#define pat_cr_2_paf(pat_cr, n) ((((uint64_t)(pat_cr)) >> ((n) << >> 3)) & 0xff) > > just a cosmetic change > No. The point here is that pat_cr could in principle be something like 1 + 2, which without parentheses may not expand as intended (in this case because of the nearby cast). Then there's also the style fix on n > >> /* Effective mm type lookup table, according to MTRR and PAT. */ >> static const uint8_t mm_type_tbl[MTRR_NUM_TYPES][X86_NUM_MT] = { >> diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c >> index 4bb1c7505534..72c7bdbfcd02 100644 >> --- a/xen/arch/x86/hvm/rtc.c >> +++ b/xen/arch/x86/hvm/rtc.c >> @@ -45,7 +45,7 @@ >> #define vrtc_domain(x) (container_of(x, struct pl_time, >> vrtc)->domain) >> #define vrtc_vcpu(x) (pt_global_vcpu_target(vrtc_domain(x))) >> #define epoch_year 1900 >> -#define get_year(x) (x + epoch_year) >> +#define get_year(x) ((x) + epoch_year) >> >> enum rtc_mode { >> rtc_mode_no_ack, >> diff --git a/xen/arch/x86/include/asm/hvm/save.h >> b/xen/arch/x86/include/asm/hvm/save.h >> index 8149aa113cb4..ec8de029319d 100644 >> --- a/xen/arch/x86/include/asm/hvm/save.h >> +++ b/xen/arch/x86/include/asm/hvm/save.h >> @@ -50,7 +50,7 @@ int _hvm_check_entry(struct hvm_domain_context *h, >> HVM_SAVE_LENGTH(x), true) == 0 ) \ >> { \ >> ptr = &(h)->data[(h)->cur]; \ >> - h->cur += HVM_SAVE_LENGTH(x); \ >> + (h)->cur += HVM_SAVE_LENGTH(x); \ >> } \ >> ptr; }) >> >> -- >> 2.34.1 >>
On 16.05.2024 01:18, Stefano Stabellini wrote: > On Wed, 15 May 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> > > >> --- >> xen/arch/x86/hvm/mtrr.c | 2 +- >> xen/arch/x86/hvm/rtc.c | 2 +- >> xen/arch/x86/include/asm/hvm/save.h | 2 +- >> 3 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c >> index 32f74c1db03b..1079851f70ed 100644 >> --- a/xen/arch/x86/hvm/mtrr.c >> +++ b/xen/arch/x86/hvm/mtrr.c >> @@ -16,7 +16,7 @@ >> #include <public/hvm/e820.h> >> >> /* Get page attribute fields (PAn) from PAT MSR. */ >> -#define pat_cr_2_paf(pat_cr,n) ((((uint64_t)pat_cr) >> ((n)<<3)) & 0xff) >> +#define pat_cr_2_paf(pat_cr, n) ((((uint64_t)(pat_cr)) >> ((n) << 3)) & 0xff) > > just a cosmetic change Not really, as Nicola already pointed out. However, there's an excess pair of parentheses which better would have been re-arranged rather than adding yet another pair: #define pat_cr_2_paf(pat_cr, n) (((uint64_t)(pat_cr) >> ((n) << 3)) & 0xff) Preferably with that (which I'll likely take the liberty of adjusting while committing) Acked-by: Jan Beulich <jbeulich@suse.com> Jan
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c index 32f74c1db03b..1079851f70ed 100644 --- a/xen/arch/x86/hvm/mtrr.c +++ b/xen/arch/x86/hvm/mtrr.c @@ -16,7 +16,7 @@ #include <public/hvm/e820.h> /* Get page attribute fields (PAn) from PAT MSR. */ -#define pat_cr_2_paf(pat_cr,n) ((((uint64_t)pat_cr) >> ((n)<<3)) & 0xff) +#define pat_cr_2_paf(pat_cr, n) ((((uint64_t)(pat_cr)) >> ((n) << 3)) & 0xff) /* Effective mm type lookup table, according to MTRR and PAT. */ static const uint8_t mm_type_tbl[MTRR_NUM_TYPES][X86_NUM_MT] = { diff --git a/xen/arch/x86/hvm/rtc.c b/xen/arch/x86/hvm/rtc.c index 4bb1c7505534..72c7bdbfcd02 100644 --- a/xen/arch/x86/hvm/rtc.c +++ b/xen/arch/x86/hvm/rtc.c @@ -45,7 +45,7 @@ #define vrtc_domain(x) (container_of(x, struct pl_time, vrtc)->domain) #define vrtc_vcpu(x) (pt_global_vcpu_target(vrtc_domain(x))) #define epoch_year 1900 -#define get_year(x) (x + epoch_year) +#define get_year(x) ((x) + epoch_year) enum rtc_mode { rtc_mode_no_ack, diff --git a/xen/arch/x86/include/asm/hvm/save.h b/xen/arch/x86/include/asm/hvm/save.h index 8149aa113cb4..ec8de029319d 100644 --- a/xen/arch/x86/include/asm/hvm/save.h +++ b/xen/arch/x86/include/asm/hvm/save.h @@ -50,7 +50,7 @@ int _hvm_check_entry(struct hvm_domain_context *h, HVM_SAVE_LENGTH(x), true) == 0 ) \ { \ ptr = &(h)->data[(h)->cur]; \ - h->cur += HVM_SAVE_LENGTH(x); \ + (h)->cur += HVM_SAVE_LENGTH(x); \ } \ ptr; })
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/mtrr.c | 2 +- xen/arch/x86/hvm/rtc.c | 2 +- xen/arch/x86/include/asm/hvm/save.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)