Re: [PATCH 2/2] xen/arm: asm/atomic.h: Fix MISRA C 2012 Rule 2.5 violation

[Julien Grall <julien@xxxxxxx>]

On 28/07/2022 08:57, Bertrand Marquis wrote:
> Hi Julien,
Hi Bertrand,

> > On 27 Jul 2022, at 16:46, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi Xenia,
> >
> > On 27/07/2022 16:32, Xenia Ragiadakou wrote:
> > > Remove unused macro atomic_xchg().
> > > Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> > > ---
> > > 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.
Cheers,

--
Julien Grall