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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

>>>> 
>>>> 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?
 From a cursory glace I can see equality comparisons and
as arguments to the array subscript operator, which are both compliant.

>> 
>> So we need either (int) or (unsigned int). The question is which one
>> between the two, and theoretically (unsigned int) is better but due to
>> the reasons above (int) is the simplest choice.
>> 
>> Yes, instead of the cast we can also add a SAF-x comment, which refers
>> to a deviation that says something along the lines "we could fix this
>> with a cast but we prefer a deviation because it makes the code easier
>> to read".
>> 
>> In general my personal preference would be to use a cast, because we
>> shouldn't implicitly convert enums to integers.
> 
> See above. I'd like to understand whether we are going to sprinkle the
> code with cast. If so, I am afraid I am against this solution.
> 
> Cheers,

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