On 06/10/2023 11:34, Julien Grall wrote:
Hi Nicola,
On 06/10/2023 09:26, Nicola Vetrini wrote:
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'.
A comment in BITS_TO_LONGS is added to make it clear that
values passed are meant to be positive.
I am confused. If the value is meant to be positive. Then...
Signed-off-by: Nicola Vetrini <[email protected]>
---
xen/include/xen/iommu.h | 2 +-
xen/include/xen/types.h | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 0e747b0bbc1c..34aa0b9b5b81 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -360,7 +360,7 @@ struct domain_iommu {
#endif
/* Features supported by the IOMMU */
- DECLARE_BITMAP(features, IOMMU_FEAT_count);
+ DECLARE_BITMAP(features, (int)IOMMU_FEAT_count);
... why do we cast to (int) rather than (unsigned int)? Also, I think
this cast deserve a comment on top because this is not a very obvious
one.
/* Does the guest share HAP mapping with the IOMMU? */
bool hap_pt_share;
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index aea259db1ef2..936e83d333a0 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -22,6 +22,7 @@ typedef signed long ssize_t;
typedef __PTRDIFF_TYPE__ ptrdiff_t;
+/* Users of this macro are expected to pass a positive value */
#define BITS_TO_LONGS(bits) \
(((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
#define DECLARE_BITMAP(name,bits) \
Cheers,
See [1] for the reason why I did so. I should have mentioned that in the
commit notes, sorry.
In short, making BITS_TO_LONGS essentially unsigned would cause a
cascade of build errors and
possibly other essential type violations. If this is to be fixed that
way, the effort required
is far greater. Either way, a comment on top of can be added, along the
lines of:
Leaving this as an enum would violate MISRA C:2012 Rule 10.1
[1]
https://lore.kernel.org/xen-devel/[email protected]/
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
