On 06.09.2023 17:57, Nicola Vetrini wrote: > On 05/09/2023 10:33, Jan Beulich wrote: >> On 05.09.2023 10:20, Nicola Vetrini wrote: >>> 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. >> >> It'll need looking closely, yes, but I expect that actually wants to be >> an >> unsigned constant. I wouldn't be surprised if some use of >> DECLARE_BITMAP() >> appeared (or already existed) where the 2nd argument involves sizeof() >> in >> some way. >> > > I think there's one with ARRAY_SIZE. In my opinion this can be resolved > as follows: > > #define BYTES_PER_LONG (1U << LONG_BYTEORDER) // the essential type gets > from signed to unsigned > > #define BITS_TO_LONGS(bits) \ > (((unsigned long long)(bits)+BITS_PER_LONG-1U)/BITS_PER_LONG) // > same here
Except, as said before, I consider any kind of cast on "bits" latently problematic. Jan
