Re: [XEN PATCH v3 3/3] xen: fix violations of MISRA C:2012 Rule 3.1

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 29 Jun 2023, Luca Fancellu wrote:
> > On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> wrote:
> > 
> > In the files modified by this patch there are a few occurrences of nested 
> > '//'
> > character sequences inside C-style comment blocks, which violate Rule 3.1.
> > The patch aims to resolve those by removing the nested comments.
> > 
> > In the file 'xen/common/xmalloc_tlsf.c' the comment has been replaces by
> 
> Typo: s/replaces/replaced/
> 
> > an ASSERT, to ensure that the invariant in the comment is enforced.
> > 
> > In the file 'xen/include/xen/atomic.h' the nested comment has been removed,
> > since the code sample is already explained by the preceding comment.
> > 
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> 
> Same as patch 2, missing “---"
> 
> > Changes:
> > - Resending the patch with the right maintainers in CC.
> > Changes in V2:
> > - Split the patch into a series and reworked the fix;
> > - Apply the fix to the arm32 `flushtlb.h' file, for consistency.
> > Changes in V3:
> > - Replaced commmented-out 'if' with an ASSERT( *fl >= 0 ).
> > ---
> > xen/common/xmalloc_tlsf.c | 4 +---
> > xen/include/xen/atomic.h  | 2 +-
> > 2 files changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> > index 75bdf18c4e..95affcc571 100644
> > --- a/xen/common/xmalloc_tlsf.c
> > +++ b/xen/common/xmalloc_tlsf.c
> > @@ -140,9 +140,7 @@ static inline void MAPPING_SEARCH(unsigned long *r, int 
> > *fl, int *sl)
> >         *fl = flsl(*r) - 1;
> >         *sl = (*r >> (*fl - MAX_LOG2_SLI)) - MAX_SLI;
> >         *fl -= FLI_OFFSET;
> > -        /*if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
> > -         *fl = *sl = 0;
> > -         */
> > +        ASSERT( *fl >= 0 );
> 
> I’ve checked the codebase for usage of ASSERT, but I’ve not seen use of it 
> with spaces
> before and after the condition (like our if conditions) so I think they can 
> be dropped.

Yes, that's right. I am OK with this patch but I think we should wait
for Jan's ack to be sure.

An alternative that I feel more comfortable in Acking myself because it
doesn't change the semantics of this code would be to change the 3 lines
of code above with this:

/*
 * ; FL will be always >0!
 * if ((*fl -= FLI_OFFSET) < 0)
 *     fl = *sl = 0;
 */