Re: [XEN PATCH][for-4.19 8/9] xen/types: address Rule 10.1 for DECLARE_BITMAP use

[Julien Grall <julien@xxxxxxx>]

On 10/10/2023 13:07, Nicola Vetrini wrote:
> > > > > I agree on everything Julien's wrote and I was about to suggest to 
> > > > > use a
> > > > > SAF comment to suppress the warning because it is clearer than a int
> > > > > cast.
> > > > >
> > > > > But then I realized that even if BITS_TO_LONGS was fixed, wouldn't 
> > > > > still
> > > > > we have a problem because IOMMU_FEAT_count is an enum?
> > > > >
> > > > > Is it the case that IOMMU_FEAT_count would have to be cast regardless,
> > > > > either to int or unsigned int or whatever to be used in 
> > > > > DECLARE_BITMAP?
> > > > >
> > > > > So we have 2 problems here: one problem is DECLARE_BITMAP taking int
> > > > > instead of unsigned int, and another problem is IOMMU_FEAT_count being
> > > > > an enum.
> > > > >
> > > > > If I got it right, then I would go with the cast to int (like done in
> > > > > this patch) with a decent comment on top of it.
> > > > I might be missing something here. But why should we use cast rather 
> > > > than /*
> > > > SAF-X */ just above? I would have expected we wanted to highlight the
> > > > violation rather than hiding it.
> > > My understanding is that the cast is required when converting an enum
> > > type to an integer type and vice versa. The idea is that we shouldn't do
> > > implicit conversions as they are error prone, only explicit conversions
> > > are allowed between enum and integers.
> > We have a lot of places in Xen using implicit conversion from enum to
> > integer (for instance in the P2M code for p2m_type_t). Does ECLAIR
> > report all of them? If not, then why?
> Can you give some pointers as to where this enum is used in arithmetic 
> operations?
I can't think of any right now. But reply from Stefano suggested that it 
was necessary anytime we were using 'enum' as an '(unsigned) int'.
>  From a cursory glace I can see equality comparisons and
> as arguments to the array subscript operator, which are both compliant.
How about assigning an enum to an '(unsigned) int : X'? (where X is the 
number of bits.
Cheers,

--
Julien Grall