Re: [XEN PATCH][for-next][for-4.19 v2 1/8] xen/include: add macro LOWEST_BIT

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 20 Oct 2023, Luca Fancellu wrote:
> > On 19 Oct 2023, at 20:58, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> > +Luca
> > 
> >>>>>> --- a/xen/include/xen/macros.h
> >>>>>> +++ b/xen/include/xen/macros.h
> >>>>>> @@ -8,8 +8,10 @@
> >>>>>> #define DIV_ROUND(n, d) (((n) + (d) / 2) / (d))
> >>>>>> #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
> >>>>>> 
> >>>>>> -#define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
> >>>>>> -#define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
> >>>>> 
> >>>>> a SAF-<n>-safe comment here?
> >>>>> 
> >>>> 
> >>>> One reason is that now that violations only belonging to tool
> >>>> configurations
> >>>> and similar are documented in docs/misra/deviations.rst (committed in
> >>>> Stefano's
> >>>> branch for-4.19 [1]).
> >>> 
> >>> But tool configuration means every analysis tool needs configuring
> >>> separately. That's why the comment tagging scheme was decided to be
> >>> preferred, iirc.
> >>> 
> >>>> Also, there were disagreements on the SAF naming
> >>>> scheme, and
> >>>> patches like those would not be accepted at the moment.
> >>> 
> >>> Well, that needs resolving. The naming there shouldn't lead to patches
> >>> being accepted that later may need redoing.
> >>> 
> >>> Jan
> >> 
> >> While this is true, in this case I'm not willing to deviate with a SAF, 
> >> given
> >> that
> >> some ECLAIR-specific configuration would be needed anyways, given that I'm
> >> deviating a macro definition, rather than the line where it's actually used
> >> (and maybe other tools would need
> >> that as well).
> > 
> > Did I get it right that the problem with using SAF in this case is that
> > it wouldn't be sufficient to add a SAF comment on top of the MACRO
> > definition, but we would need a SAF comment on top of every MACRO
> > invocation?
> > 
> > If so, then not just for this MACRO but in general basically we have to
> > use deviations.rst.
> > 
> > Luca, do you know what would be the behavior for cppcheck and/or
> > Coverity? I imagine it will be the same and they would also need a
> > deviation at every MACRO invocation, not just the definition?
> 
> Seems that cppcheck reports at the macro definition, instead Coverity reports
> at the macro invocation.

Does that mean that a /* SAF-xx-safe */ comment would work at the MACRO
definition for cppcheck but not for Coverity?

If so, then I wonder if cppcheck's behavior is what we would like to
have, but from a code compliance point of view it is the least reliable,
so that's the reason why both Coverity and ECLAIR don't implement it. In
terms of correctness of the implementation we know cppcheck is lagging a
bit behind...