| Message ID | f94b0c9411937a4003b86b5850b6d4ad91dff5d5.1697123806.git.nicola.vetrini@bugseng.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | address violations of MISRA C:2012 Rule 10.1 | expand |
On 12.10.2023 17:28, Nicola Vetrini wrote: > --- a/xen/include/xen/types.h > +++ b/xen/include/xen/types.h > @@ -22,6 +22,14 @@ typedef signed long ssize_t; > > typedef __PTRDIFF_TYPE__ ptrdiff_t; > > +/* > + * Users of this macro are expected to pass a positive value. Is passing 0 going to cause any issues? > + * Eventually, this should become an unsigned quantity, but this > + * requires fixing various uses of this macro and BITS_PER_LONG in signed > + * contexts, such as type-safe 'min' macro uses, which give rise to build errors > + * when the arguments have differing signedness, due to the build flags used. > + */ I'm not convinced of the usefulness of this part of the comment. Jan > #define BITS_TO_LONGS(bits) \ > (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG) > #define DECLARE_BITMAP(name,bits) \
On 16/10/2023 17:49, Jan Beulich wrote: > On 12.10.2023 17:28, Nicola Vetrini wrote: >> --- a/xen/include/xen/types.h >> +++ b/xen/include/xen/types.h >> @@ -22,6 +22,14 @@ typedef signed long ssize_t; >> >> typedef __PTRDIFF_TYPE__ ptrdiff_t; >> >> +/* >> + * Users of this macro are expected to pass a positive value. > > Is passing 0 going to cause any issues? > I don't think so, even if that wouldn't make much sense. Given that the usage of the zero lenght array extension is documented, that shouldn't be a concern either. >> + * Eventually, this should become an unsigned quantity, but this >> + * requires fixing various uses of this macro and BITS_PER_LONG in >> signed >> + * contexts, such as type-safe 'min' macro uses, which give rise to >> build errors >> + * when the arguments have differing signedness, due to the build >> flags used. >> + */ > > I'm not convinced of the usefulness of this part of the comment. > > Jan > Isn't it useful to record why it was left as-is, and what should be done about it? If it's not, this can be dropped on commit, in my opinion.
diff --git a/docs/misra/safe.json b/docs/misra/safe.json index 39c5c056c7d4..952324f85cf9 100644 --- a/docs/misra/safe.json +++ b/docs/misra/safe.json @@ -20,6 +20,14 @@ }, { "id": "SAF-2-safe", + "analyser": { + "eclair": "MC3R1.R10.1" + }, + "name": "MC3R1.R10.1: use of an enumeration constant in an arithmetic operation", + "text": "This violation can be fixed with a cast to (int) of the enumeration constant, but a deviation was chosen due to code readability (see also the comment in BITS_TO_LONGS)." + }, + { + "id": "SAF-3-safe", "analyser": {}, "name": "Sentinel", "text": "Next ID to be used" diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h index 0e747b0bbc1c..d5c25770915b 100644 --- a/xen/include/xen/iommu.h +++ b/xen/include/xen/iommu.h @@ -360,6 +360,7 @@ struct domain_iommu { #endif /* Features supported by the IOMMU */ + /* SAF-2-safe enum constant in arithmetic operation */ DECLARE_BITMAP(features, IOMMU_FEAT_count); /* Does the guest share HAP mapping with the IOMMU? */ diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h index aea259db1ef2..50171ea1ad28 100644 --- a/xen/include/xen/types.h +++ b/xen/include/xen/types.h @@ -22,6 +22,14 @@ typedef signed long ssize_t; typedef __PTRDIFF_TYPE__ ptrdiff_t; +/* + * Users of this macro are expected to pass a positive value. + * + * Eventually, this should become an unsigned quantity, but this + * requires fixing various uses of this macro and BITS_PER_LONG in signed + * contexts, such as type-safe 'min' macro uses, which give rise to build errors + * when the arguments have differing signedness, due to the build flags used. + */ #define BITS_TO_LONGS(bits) \ (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG) #define DECLARE_BITMAP(name,bits) \
Given its use in the declaration 'DECLARE_BITMAP(features, IOMMU_FEAT_count)' the argument 'bits' has essential type 'enum iommu_feature', which is not allowed by the Rule as an operand to the addition operator in macro 'BITS_TO_LONGS'. This construct is deviated with a deviation comment. Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com> --- docs/misra/safe.json | 8 ++++++++ xen/include/xen/iommu.h | 1 + xen/include/xen/types.h | 8 ++++++++ 3 files changed, 17 insertions(+)