Hi Nicola,
On 06/10/2023 11:34, Nicola Vetrini wrote:
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 is
indeed 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.
Just to clarify, you are saying that the following would not work for
Eclair:
/* SAF-XXX */
#define LOWEST_POW2()
Instead you would need the following:
void foo()
{
/* SAF-XXX */
LOWEST()
}
Am I correct? If so, would something like below (untested) work?
#define LOWEST_POW2(...) ({ \
/* SAFE-XXX */ \
...
})
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).
I don't think we need to know whether a tool detects it. We only need to
know whether this is violation to MISRA. If this is one, then this is a
call to have a marker in the code.
If this is a false positive, then adding the deviation in the tool
configuration is best (unless there are multiple tools affected).
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.
If the comment is around the macro there are limited chance that this
will be missed. But if you are worried about code movement, you should
be worried about macro renaming with your approach (one may not know
Eclair has a deviation) and/or function with the same name.
I am curious to know what the other thinks.
Cheers,
--
Julien Grall
