| Message ID | 20220620070245.77979-6-michal.orzel@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MISRA C 2012 8.1 rule fixes | expand |
Hi Michal, On 20/06/2022 08:02, Michal Orzel wrote: > According to MISRA C 2012 Rule 8.1, types shall be explicitly > specified. Fix all the findings reported by cppcheck with misra addon > by substituting implicit type 'unsigned' to explicit 'unsigned int'. > > Bump sysctl interface version. The sysctl version should only be bumped if the ABI has changed. AFAICT switching from "unsigned" to "unsigned" will not modify it, so I don't think this is necessary. > > Signed-off-by: Michal Orzel <michal.orzel@arm.com> > --- > xen/include/public/physdev.h | 4 ++-- > xen/include/public/sysctl.h | 10 +++++----- > 2 files changed, 7 insertions(+), 7 deletions(-) > > diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h > index d271766ad0..a2ca0ee564 100644 > --- a/xen/include/public/physdev.h > +++ b/xen/include/public/physdev.h > @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext { > /* IN */ > uint8_t bus; > uint8_t devfn; > - unsigned is_extfn; > - unsigned is_virtfn; > + unsigned int is_extfn; > + unsigned int is_virtfn; > struct { > uint8_t bus; > uint8_t devfn; > diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h > index b0a4af8789..a2a762fe46 100644 > --- a/xen/include/public/sysctl.h > +++ b/xen/include/public/sysctl.h > @@ -35,7 +35,7 @@ > #include "domctl.h" > #include "physdev.h" > > -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014 > +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015 > > /* > * Read console content from Xen buffer ring. > @@ -644,18 +644,18 @@ struct xen_sysctl_credit_schedule { > /* Length of timeslice in milliseconds */ > #define XEN_SYSCTL_CSCHED_TSLICE_MAX 1000 > #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1 > - unsigned tslice_ms; > - unsigned ratelimit_us; > + unsigned int tslice_ms; > + unsigned int ratelimit_us; > /* > * How long we consider a vCPU to be cache-hot on the > * CPU where it has run (max 100ms, in microseconds) > */ > #define XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US (100 * 1000) > - unsigned vcpu_migr_delay_us; > + unsigned int vcpu_migr_delay_us; > }; > > struct xen_sysctl_credit2_schedule { > - unsigned ratelimit_us; > + unsigned int ratelimit_us; > }; > > /* XEN_SYSCTL_scheduler_op */ Cheers,
On 20/06/2022 10:54, Julien Grall wrote: > Hi Michal, > > On 20/06/2022 08:02, Michal Orzel wrote: >> According to MISRA C 2012 Rule 8.1, types shall be explicitly >> specified. Fix all the findings reported by cppcheck with misra addon >> by substituting implicit type 'unsigned' to explicit 'unsigned int'. >> >> Bump sysctl interface version. > > The sysctl version should only be bumped if the ABI has changed. > AFAICT switching from "unsigned" to "unsigned" will not modify it, so > I don't think this is necessary. Yeah. No need to bump the interface version for this. ~Andrew
Hi Julien, On 20.06.2022 11:54, Julien Grall wrote: > Hi Michal, > > On 20/06/2022 08:02, Michal Orzel wrote: >> According to MISRA C 2012 Rule 8.1, types shall be explicitly >> specified. Fix all the findings reported by cppcheck with misra addon >> by substituting implicit type 'unsigned' to explicit 'unsigned int'. >> >> Bump sysctl interface version. > > The sysctl version should only be bumped if the ABI has changed. AFAICT switching from "unsigned" to "unsigned" will not modify it, so I don't think this is necessary. Sure, I can remove that in v2 but first I'd like to wait at least for xsm patch to be reviewed. Also as these patches are not dependent from each other, do you think it is worth respinning the reviewed ones? Cheers, Michal
On 21/06/2022 09:43, Michal Orzel wrote: > Hi Julien, Hi Michal, > > On 20.06.2022 11:54, Julien Grall wrote: >> Hi Michal, >> >> On 20/06/2022 08:02, Michal Orzel wrote: >>> According to MISRA C 2012 Rule 8.1, types shall be explicitly >>> specified. Fix all the findings reported by cppcheck with misra addon >>> by substituting implicit type 'unsigned' to explicit 'unsigned int'. >>> >>> Bump sysctl interface version. >> >> The sysctl version should only be bumped if the ABI has changed. AFAICT switching from "unsigned" to "unsigned" will not modify it, so I don't think this is necessary. > > Sure, I can remove that in v2 but first I'd like to wait at least for xsm patch to be reviewed. > Also as these patches are not dependent from each other, do you think it is worth respinning the reviewed ones? I would suggest to wait until you get input on all the patches before respinning. Cheers,
On 20.06.2022 09:02, Michal Orzel wrote: > --- a/xen/include/public/physdev.h > +++ b/xen/include/public/physdev.h > @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext { > /* IN */ > uint8_t bus; > uint8_t devfn; > - unsigned is_extfn; > - unsigned is_virtfn; > + unsigned int is_extfn; > + unsigned int is_virtfn; It is wrong for us to use unsigned (or unsigned int) here and in sysctl.h. It should be uint32_t instead, and I think this is a great opportunity to correct that mistake. Jan
On 22.06.2022 12:16, Jan Beulich wrote: > On 20.06.2022 09:02, Michal Orzel wrote: >> --- a/xen/include/public/physdev.h >> +++ b/xen/include/public/physdev.h >> @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext { >> /* IN */ >> uint8_t bus; >> uint8_t devfn; >> - unsigned is_extfn; >> - unsigned is_virtfn; >> + unsigned int is_extfn; >> + unsigned int is_virtfn; > > It is wrong for us to use unsigned (or unsigned int) here and in sysctl.h. > It should be uint32_t instead, and I think this is a great opportunity to > correct that mistake. > That is perfectly fine for me to do. > Jan Cheers, Michal
diff --git a/xen/include/public/physdev.h b/xen/include/public/physdev.h index d271766ad0..a2ca0ee564 100644 --- a/xen/include/public/physdev.h +++ b/xen/include/public/physdev.h @@ -211,8 +211,8 @@ struct physdev_manage_pci_ext { /* IN */ uint8_t bus; uint8_t devfn; - unsigned is_extfn; - unsigned is_virtfn; + unsigned int is_extfn; + unsigned int is_virtfn; struct { uint8_t bus; uint8_t devfn; diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h index b0a4af8789..a2a762fe46 100644 --- a/xen/include/public/sysctl.h +++ b/xen/include/public/sysctl.h @@ -35,7 +35,7 @@ #include "domctl.h" #include "physdev.h" -#define XEN_SYSCTL_INTERFACE_VERSION 0x00000014 +#define XEN_SYSCTL_INTERFACE_VERSION 0x00000015 /* * Read console content from Xen buffer ring. @@ -644,18 +644,18 @@ struct xen_sysctl_credit_schedule { /* Length of timeslice in milliseconds */ #define XEN_SYSCTL_CSCHED_TSLICE_MAX 1000 #define XEN_SYSCTL_CSCHED_TSLICE_MIN 1 - unsigned tslice_ms; - unsigned ratelimit_us; + unsigned int tslice_ms; + unsigned int ratelimit_us; /* * How long we consider a vCPU to be cache-hot on the * CPU where it has run (max 100ms, in microseconds) */ #define XEN_SYSCTL_CSCHED_MGR_DLY_MAX_US (100 * 1000) - unsigned vcpu_migr_delay_us; + unsigned int vcpu_migr_delay_us; }; struct xen_sysctl_credit2_schedule { - unsigned ratelimit_us; + unsigned int ratelimit_us; }; /* XEN_SYSCTL_scheduler_op */
According to MISRA C 2012 Rule 8.1, types shall be explicitly specified. Fix all the findings reported by cppcheck with misra addon by substituting implicit type 'unsigned' to explicit 'unsigned int'. Bump sysctl interface version. Signed-off-by: Michal Orzel <michal.orzel@arm.com> --- xen/include/public/physdev.h | 4 ++-- xen/include/public/sysctl.h | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-)