Hi Julien, > On 28 Jul 2022, at 11:21, Julien Grall <jul...@xen.org> wrote: > > > > On 28/07/2022 10:45, Bertrand Marquis wrote: >>> On 28 Jul 2022, at 10:35, Julien Grall <jul...@xen.org> wrote: >>> >>> >>> >>> On 28/07/2022 08:57, Bertrand Marquis wrote: >>>> Hi Julien, >>> >>> Hi Bertrand, >>> >>>>> On 27 Jul 2022, at 16:46, Julien Grall <jul...@xen.org> wrote: >>>>> >>>>> Hi Xenia, >>>>> >>>>> On 27/07/2022 16:32, Xenia Ragiadakou wrote: >>>>>> Remove unused macro atomic_xchg(). >>>>>> Signed-off-by: Xenia Ragiadakou <burzalod...@gmail.com> >>>>>> --- >>>>>> xen/arch/arm/include/asm/atomic.h | 2 -- >>>>>> 1 file changed, 2 deletions(-) >>>>>> diff --git a/xen/arch/arm/include/asm/atomic.h >>>>>> b/xen/arch/arm/include/asm/atomic.h >>>>>> index f5ef744b4b..a2dc125291 100644 >>>>>> --- a/xen/arch/arm/include/asm/atomic.h >>>>>> +++ b/xen/arch/arm/include/asm/atomic.h >>>>>> @@ -223,8 +223,6 @@ static inline int atomic_add_unless(atomic_t *v, int >>>>>> a, int u) >>>>>> return __atomic_add_unless(v, a, u); >>>>>> } >>>>>> -#define atomic_xchg(v, new) (xchg(&((v)->counter), new)) >>>>>> - >>>>> >>>>> While I agree this is unused today, the wrapper is quite trivial and part >>>>> of the generic API (x86 also provides one). So I am not in favor of >>>>> removing it just to please MISRA. >>>>> >>>>> That said, if Bertrand and Stefano agrees with removing it then you >>>>> should also remove the x86 version to avoid inconsistency. >>>> I think we can keep this and maybe add a comment on top to document a >>>> known violation: >>>> /* TODO: MISRA_VIOLATION 2.5 */ >>> >>> While I am fine with this goal of the comment (i.e. indicating where Xen is >>> not MISRA compliant), I think this is one place where I would rather not >>> want one because it can get stale if someones decide to use the function. >> I think the one doing that will have to update the comment otherwise we will >> never manage to have an analysis without findings. > > I was under the impression that Xen will never officially follow some of the > MISRA rules. So I would expect the tools to be able to detect such cases so > we don't have to add a comment for every deviation on something we will never > support. > >> Having those kind of comments in the code for violation also means that they >> must be updated if the violation is solved. > > Right, but for thing like unused function, this is quite easy to miss by both > the developer and reviewers. So we are going to end up to comments for > nothing. > >> Maybe we will need a run ignoring those to identify possible violations >> which are not violations anymore but this might be hard to do. > > TBH, I think it would be best if we can teach the tools to ignore certain > rules.
Definitely it is possible to instruct the tool to ignore this you are right and for 2.5 we should (for some reason I was under the impression that we said we would follow 2.5 but accept deviations). @Xenia: please ignore and do not add a comment for this. I think we will need to distinguish 2 kind of not following: - not following at all (disable in the tools) - accepting some deviations (documented in the code) As much as we can, I think we should target the second unless we have a lot of violations. Cheers Bertrand > > Cheers, > > -- > Julien Grall
