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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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)