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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.06.2023 09:42, Nicola Vetrini wrote:
> The xen sources contain several violations of Rule 3.1 from MISRA C:2012,
> whose headline states:
> "The character sequences '/*' and '//' shall not be used within a comment".
> 
> Most of the violations are due to the presence of links to webpages within
> C-style comment blocks, such as:
> 
> xen/arch/arm/include/asm/smccc.h:37.1-41.3
> /*
>  * This file provides common defines for ARM SMC Calling Convention as
>  * specified in
>  * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
>  */
> 
> In this case, we propose to deviate all of these occurrences with a
> project deviation to be captured by a tool configuration.
> 
> There are, however, a few other violations that do not fall under this
> category, namely:
> 
> 1. in file "xen/arch/arm/include/asm/arm64/flushtlb.h" we propose to
> avoid the usage of a nested comment;
> 2. in file "xen/common/xmalloc_tlsf.c" we propose to substitute the
> commented-out if statement with a "#if 0 .. #endif;
> 3. in file "xen/include/xen/atomic.h" and
> "xen/drivers/passthrough/arm/smmu-v3.c" we propose to split the C-style
> comment containing the nested comment into two doxygen comments, clearly
> identifying the second as a code sample. This can then be captured with a
> project deviation by a tool configuration.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> ---
> Changes:
> - Resending the patch with the right maintainers in CC.

But without otherwise addressing comments already given, afaics. One more
remark:

> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -140,9 +140,10 @@ 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;
> -         */
> +#if 0
> +        if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
> +        fl = *sl = 0;

You want to get indentation right here, and you don't want to lose
the indirection on fl.

> +#endif
>          *r &= ~t;
>      }
>  }

If you split this to 4 patches, leaving the URL proposal in just
the cover letter, then I think this one change (with the adjustments)
could go in right away. Similarly I expect the arm64/flushtlb.h
change could be ack-ed right away by an Arm maintainer.

Jan