From: Gianluca Luparini <gianluca.luparini@bugseng.com>
The xen sources contains violations of MISRA C:2012 Rule 7.2 whose
headline states:
"A 'u' or 'U' suffix shall be applied to all integer constants
that are represented in an unsigned type".
Add the 'U' suffix to integers literals with unsigned type and also to other
literals used in the same contexts or near violations, when their positive
nature is immediately clear. The latter changes are done for the sake of
uniformity.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com>
---
Changes in v2:
- minor change to commit title
- change commit message
- correct macros code style
---
xen/include/public/io/ring.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h
index 025939278b..0cae4367be 100644
--- a/xen/include/public/io/ring.h
+++ b/xen/include/public/io/ring.h
@@ -36,11 +36,11 @@
typedef unsigned int RING_IDX;
/* Round a 32-bit unsigned constant down to the nearest power of two. */
-#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1))
-#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x))
-#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x))
-#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x))
-#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x))
+#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1))
+#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x))
+#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x))
+#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x))
+#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x))
/*
* Calculate size of a shared ring, given the total available space for the
--
2.41.0On Wed, 5 Jul 2023, Simone Ballarin wrote: > From: Gianluca Luparini <gianluca.luparini@bugseng.com> > > The xen sources contains violations of MISRA C:2012 Rule 7.2 whose > headline states: > "A 'u' or 'U' suffix shall be applied to all integer constants > that are represented in an unsigned type". > > Add the 'U' suffix to integers literals with unsigned type and also to other > literals used in the same contexts or near violations, when their positive > nature is immediately clear. The latter changes are done for the sake of > uniformity. > > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> > Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > Changes in v2: > - minor change to commit title > - change commit message > - correct macros code style > --- > xen/include/public/io/ring.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h > index 025939278b..0cae4367be 100644 > --- a/xen/include/public/io/ring.h > +++ b/xen/include/public/io/ring.h > @@ -36,11 +36,11 @@ > typedef unsigned int RING_IDX; > > /* Round a 32-bit unsigned constant down to the nearest power of two. */ > -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) > -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) > -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) > -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) > -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) > +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) > +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) > +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) > +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) > +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) > > /* > * Calculate size of a shared ring, given the total available space for the > -- > 2.41.0 >
On 05.07.23 17:26, Simone Ballarin wrote: > From: Gianluca Luparini <gianluca.luparini@bugseng.com> > > The xen sources contains violations of MISRA C:2012 Rule 7.2 whose > headline states: > "A 'u' or 'U' suffix shall be applied to all integer constants > that are represented in an unsigned type". > > Add the 'U' suffix to integers literals with unsigned type and also to other > literals used in the same contexts or near violations, when their positive > nature is immediately clear. The latter changes are done for the sake of > uniformity. > > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> > Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> > --- > Changes in v2: > - minor change to commit title > - change commit message > - correct macros code style > --- > xen/include/public/io/ring.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h > index 025939278b..0cae4367be 100644 > --- a/xen/include/public/io/ring.h > +++ b/xen/include/public/io/ring.h > @@ -36,11 +36,11 @@ > typedef unsigned int RING_IDX; > > /* Round a 32-bit unsigned constant down to the nearest power of two. */ > -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) > -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) > -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) > -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) > -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) > +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) Shouldn't this be rather: +#define __RD2(x) (((x) & 0x00000002U) ? 0x2U : ((x) & 0x1U)) > +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) > +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) > +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) > +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) > > /* > * Calculate size of a shared ring, given the total available space for the Other than that: Reviewed-by: Juergen Gross <jgross@suse.com> Juergen
On 05.07.2023 17:33, Juergen Gross wrote: > On 05.07.23 17:26, Simone Ballarin wrote: >> From: Gianluca Luparini <gianluca.luparini@bugseng.com> >> >> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose >> headline states: >> "A 'u' or 'U' suffix shall be applied to all integer constants >> that are represented in an unsigned type". >> >> Add the 'U' suffix to integers literals with unsigned type and also to other >> literals used in the same contexts or near violations, when their positive >> nature is immediately clear. The latter changes are done for the sake of >> uniformity. >> >> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> >> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> >> --- >> Changes in v2: >> - minor change to commit title >> - change commit message >> - correct macros code style >> --- >> xen/include/public/io/ring.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h >> index 025939278b..0cae4367be 100644 >> --- a/xen/include/public/io/ring.h >> +++ b/xen/include/public/io/ring.h >> @@ -36,11 +36,11 @@ >> typedef unsigned int RING_IDX; >> >> /* Round a 32-bit unsigned constant down to the nearest power of two. */ >> -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) >> -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) >> -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) >> -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) >> -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) >> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) > > Shouldn't this be rather: > > +#define __RD2(x) (((x) & 0x00000002U) ? 0x2U : ((x) & 0x1U)) I don't think it matters much (as the comment says, the input is expected to be unsigned anyway), and I expect even the one U that was added here was only added for consistency. The sole one that really matter is imo ... >> +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) >> +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) >> +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) >> +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) ... this single one. Jan
On 06.07.23 09:43, Jan Beulich wrote: > On 05.07.2023 17:33, Juergen Gross wrote: >> On 05.07.23 17:26, Simone Ballarin wrote: >>> From: Gianluca Luparini <gianluca.luparini@bugseng.com> >>> >>> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose >>> headline states: >>> "A 'u' or 'U' suffix shall be applied to all integer constants >>> that are represented in an unsigned type". >>> >>> Add the 'U' suffix to integers literals with unsigned type and also to other >>> literals used in the same contexts or near violations, when their positive >>> nature is immediately clear. The latter changes are done for the sake of >>> uniformity. >>> >>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> >>> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> >>> --- >>> Changes in v2: >>> - minor change to commit title >>> - change commit message >>> - correct macros code style >>> --- >>> xen/include/public/io/ring.h | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h >>> index 025939278b..0cae4367be 100644 >>> --- a/xen/include/public/io/ring.h >>> +++ b/xen/include/public/io/ring.h >>> @@ -36,11 +36,11 @@ >>> typedef unsigned int RING_IDX; >>> >>> /* Round a 32-bit unsigned constant down to the nearest power of two. */ >>> -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) >>> -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) >>> -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) >>> -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) >>> -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) >>> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) >> >> Shouldn't this be rather: >> >> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2U : ((x) & 0x1U)) > > I don't think it matters much (as the comment says, the input is expected > to be unsigned anyway), and I expect even the one U that was added here > was only added for consistency. The sole one that really matter is imo ... > >>> +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) >>> +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) >>> +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) >>> +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) > > ... this single one. I agree that only the last one is really needed. But for consistency reasons I'd expect all optional "U"s to be either dropped or specified, instead of a mixture. Juergen
On 06.07.2023 09:57, Juergen Gross wrote: > On 06.07.23 09:43, Jan Beulich wrote: >> On 05.07.2023 17:33, Juergen Gross wrote: >>> On 05.07.23 17:26, Simone Ballarin wrote: >>>> From: Gianluca Luparini <gianluca.luparini@bugseng.com> >>>> >>>> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose >>>> headline states: >>>> "A 'u' or 'U' suffix shall be applied to all integer constants >>>> that are represented in an unsigned type". >>>> >>>> Add the 'U' suffix to integers literals with unsigned type and also to other >>>> literals used in the same contexts or near violations, when their positive >>>> nature is immediately clear. The latter changes are done for the sake of >>>> uniformity. >>>> >>>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> >>>> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> >>>> --- >>>> Changes in v2: >>>> - minor change to commit title >>>> - change commit message >>>> - correct macros code style >>>> --- >>>> xen/include/public/io/ring.h | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h >>>> index 025939278b..0cae4367be 100644 >>>> --- a/xen/include/public/io/ring.h >>>> +++ b/xen/include/public/io/ring.h >>>> @@ -36,11 +36,11 @@ >>>> typedef unsigned int RING_IDX; >>>> >>>> /* Round a 32-bit unsigned constant down to the nearest power of two. */ >>>> -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) >>>> -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) >>>> -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) >>>> -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) >>>> -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) >>>> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) >>> >>> Shouldn't this be rather: >>> >>> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2U : ((x) & 0x1U)) >> >> I don't think it matters much (as the comment says, the input is expected >> to be unsigned anyway), and I expect even the one U that was added here >> was only added for consistency. The sole one that really matter is imo ... >> >>>> +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) >>>> +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) >>>> +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) >>>> +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) >> >> ... this single one. > > I agree that only the last one is really needed. > > But for consistency reasons I'd expect all optional "U"s to be either dropped or > specified, instead of a mixture. Funny you should say this. Shift counts also aren't allowed to be negative ... For this reason, the pattern I see here is to have U uniformly on the lhs of the ?: operator, and nowhere else. Jan
On 06.07.23 10:43, Jan Beulich wrote: > On 06.07.2023 09:57, Juergen Gross wrote: >> On 06.07.23 09:43, Jan Beulich wrote: >>> On 05.07.2023 17:33, Juergen Gross wrote: >>>> On 05.07.23 17:26, Simone Ballarin wrote: >>>>> From: Gianluca Luparini <gianluca.luparini@bugseng.com> >>>>> >>>>> The xen sources contains violations of MISRA C:2012 Rule 7.2 whose >>>>> headline states: >>>>> "A 'u' or 'U' suffix shall be applied to all integer constants >>>>> that are represented in an unsigned type". >>>>> >>>>> Add the 'U' suffix to integers literals with unsigned type and also to other >>>>> literals used in the same contexts or near violations, when their positive >>>>> nature is immediately clear. The latter changes are done for the sake of >>>>> uniformity. >>>>> >>>>> Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> >>>>> Signed-off-by: Gianluca Luparini <gianluca.luparini@bugseng.com> >>>>> --- >>>>> Changes in v2: >>>>> - minor change to commit title >>>>> - change commit message >>>>> - correct macros code style >>>>> --- >>>>> xen/include/public/io/ring.h | 10 +++++----- >>>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h >>>>> index 025939278b..0cae4367be 100644 >>>>> --- a/xen/include/public/io/ring.h >>>>> +++ b/xen/include/public/io/ring.h >>>>> @@ -36,11 +36,11 @@ >>>>> typedef unsigned int RING_IDX; >>>>> >>>>> /* Round a 32-bit unsigned constant down to the nearest power of two. */ >>>>> -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) & 0x1)) >>>>> -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : __RD2(_x)) >>>>> -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : __RD4(_x)) >>>>> -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : __RD8(_x)) >>>>> -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : __RD16(_x)) >>>>> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2 : ((x) & 0x1)) >>>> >>>> Shouldn't this be rather: >>>> >>>> +#define __RD2(x) (((x) & 0x00000002U) ? 0x2U : ((x) & 0x1U)) >>> >>> I don't think it matters much (as the comment says, the input is expected >>> to be unsigned anyway), and I expect even the one U that was added here >>> was only added for consistency. The sole one that really matter is imo ... >>> >>>>> +#define __RD4(x) (((x) & 0x0000000cU) ? __RD2((x) >> 2) << 2 : __RD2(x)) >>>>> +#define __RD8(x) (((x) & 0x000000f0U) ? __RD4((x) >> 4) << 4 : __RD4(x)) >>>>> +#define __RD16(x) (((x) & 0x0000ff00U) ? __RD8((x) >> 8) << 8 : __RD8(x)) >>>>> +#define __RD32(x) (((x) & 0xffff0000U) ? __RD16((x) >> 16) << 16 : __RD16(x)) >>> >>> ... this single one. >> >> I agree that only the last one is really needed. >> >> But for consistency reasons I'd expect all optional "U"s to be either dropped or >> specified, instead of a mixture. > > Funny you should say this. Shift counts also aren't allowed to be negative > ... For this reason, the pattern I see here is to have U uniformly on the > lhs of the ?: operator, and nowhere else. Yes, this is one way to look at it. My view would be that (at least) the constants used for ANDing should have a uniform U attribution. In the end I'm fine with either way. I just wanted to point out a slight inconsistency with the patch. Juergen
© 2016 - 2026 Red Hat, Inc.