[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH][for-4.19 1/9] xen/include: add macro LOWEST_POW2
On 06/10/2023 12:22, Julien Grall wrote: On 06/10/2023 11:02, Nicola Vetrini wrote:On 06/10/2023 11:29, Julien Grall wrote:Hi, On 06/10/2023 09:26, Nicola Vetrini wrote:The purpose of this macro is to encapsulate the well-known expression 'x & -x', that in 2's complement architectures on unsigned integers will give 2^ffs(x), where ffs(x) is the position of the lowest set bit in x.A deviation for ECLAIR is also introduced.Can you explain why this is a deviation in ECLAIR rather than one with/* SAF-* */ (or whichever name we decide to rename to)? Is this because the code is correct from MISRA perspective but the tool is confused?The code does violate Rule 10.1 (a unary minus applied to an unsigned value is deemed inappropriate by MISRA), but rather than changing a whole lot of places where this construct is used (mainly in x86 code), the reasoning is that it makes more sense to isolate it and justify its presence by the fact that on 2's complement architectures the result isindeed correct.This is explaining to me why you are adding LOWEST_POW2(). But this doesn't explain why you are not using /* SAF-* */ on top of LOWEST_POW2(). To me, we should only use ECLAIR specific deviation when this is a shortcoming with the tool. Cheers, Because of the way ECLAIR deviation comments work implies that in most cases the real place where to put the deviation is the usage site(the so-called top expansion location of the macro). Now, for widely-used macros this is cumbersome and would clutter the code unnecessarily. It's way cleaner imo to have a single line in the configuration with a clear justification that is present in the textual output of the tool.But then there are tool interoperability considerations, that would call for standardized deviation mechanisms, if they do detect this as a violation (which I don't know). In the end, it could be done with a textual deviation, if that's preferred, but keep in mind that those are more fragile w.r.t. code movement. -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |