[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
On 10/10/2023 14:15, Julien Grall wrote: On 10/10/2023 13:13, Julien Grall wrote: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 intcast.But then I realized that even if BITS_TO_LONGS was fixed, wouldn't stillwe 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 beingan enum.If I got it right, then I would go with the cast to int (like done inthis 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 theviolation 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 conversionsare allowed between enum and integers.We have a lot of places in Xen using implicit conversion from enum tointeger (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.Obviously, right after I pressed send, I remember of one in __acpi_map_table() (x86 code).FIX_ACPI_END is an enum, assigned to an 'int' and then used in arithmetics.Cheers, A couple of remarks:- that file is part of the exclude-list.json, therefore it's not bound to be MISRA compliant right now;- assignment to a different essential type category is dealt with by Rule 10.3 (The value of an expression shall not be assigned to an object with a narrower essential type or of a different essential type category), so perhaps Luca's script does indeed capture it with gcc compilation flags.Aside from this, if you feel that a deviation comment is a better choice, I'm ok with dealing with it in this way. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |