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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 12/07/23 18:02, Luca Fancellu wrote:
> > On 12 Jul 2023, at 16:54, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> wrote:
> >
> > In the file 'xen/common/xmalloc_tlsf.c' is not clear how
> > the commented-out code should interact with the previous statement.
> > To resolve the MISRA violation generated by the nested comment
> > a #if .. #endif block with an explanatory comment substitutes
> > the earlier construct.
> >
> > 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>
> > ---
> > Following the suggestion of this message
> > https://lore.kernel.org/xen-devel/536f3049-41f7-b127-ba94-81925e34ea0f@xxxxxxxx/
> > an explanatory comment has been added.
> > ---
> > xen/common/xmalloc_tlsf.c | 13 ++++++++++---
> > xen/include/xen/atomic.h  |  2 +-
> > 2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
> > index c21bf71e88..56c3849414 100644
> > --- a/xen/common/xmalloc_tlsf.c
> > +++ b/xen/common/xmalloc_tlsf.c
> > @@ -139,10 +139,17 @@ static inline void MAPPING_SEARCH(unsigned long *r, int 
> > *fl, int *sl)
> >          *r = *r + t;
> >          *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;
> > +        /*
> > +         * It's unclear what was the purpose of the commented-out code that now
> > +         * is in the #else branch. The current form is motivated by the 
> > correction
> > +         * of a violation MISRA:C 2012 Rule 3.1
> >           */
> > +#if 1
> > +        *fl -= FLI_OFFSET;
> > +#else
> > +        if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
> In the message you linked above, you suggested to use /* FL will be always >0! 
> */, why has it changed?
> Was some comment I missed? The xen codestyle mandates the use of /* */, anyway 
> I agree that here you
> are just moving code...
> So maybe the maintainer can tell what is the best thing to do here.
You didn't miss any further comment: my suggestion was related to the 
explanatory comment, not the nested comment itself. If a better wording 
can be found for the former, no problem. As for the codestyle point: it 
does not change anything doing
"// FL will be always >0!" -> "/* FL will be always >0!  */
w.r.t. Rule 3.1 (both would have been nested comments).

Regards,

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)