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

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





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)



 


Rackspace

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