[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH] xen/types: address Rule 10.1 for macro BITS_TO_LONGS



On 05/09/2023 09:46, Jan Beulich wrote:
On 05.09.2023 09:31, 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.
Given that its value can be represented by a signed integer,
the explicit cast resolves the violation.

Wait - why would this lead to a change to BITS_TO_LONGS()? And if that
was to be changed, why plain int? I don't think negative input makes
sense there, and in principle I'd expect values beyond 4 billion to
also be permissible (even if likely no such use will ever appear in a
DECLARE_BITMAP(), but elsewhere it may make sense). Even going to
"unsigned long" may be too limiting ...


You have a point. I can think of doing it like this:
DECLARE_BITMAP(features, (int)IOMMU_FEAT_count)
on the grounds that the enum constant is representable in an int, and it does not seem likely
to get much bigger.
Having an unsigned cast requires making the whole expression
essentially unsigned, otherwise Rule 10.4 is violated because BITS_PER_LONG is essentially signed. This can be done, but it depends on how BITS_TO_LONGS will be/is used.

--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -23,7 +23,7 @@ typedef signed long ssize_t;
 typedef __PTRDIFF_TYPE__ ptrdiff_t;

 #define BITS_TO_LONGS(bits) \
-    (((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
+    (((int)(bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
 #define DECLARE_BITMAP(name,bits) \
     unsigned long name[BITS_TO_LONGS(bits)]


Furthermore, as always - if this was to be touched, please take care
of style violations (numerous missing blanks) at this occasion.


Then the whole file needs a cleanup.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.