> On 29 Jun 2023, at 20:20, Stefano Stabellini <sstabell...@kernel.org> wrote: > > On Thu, 29 Jun 2023, Luca Fancellu wrote: >>> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetr...@bugseng.com> 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.vetr...@bugseng.com> >> >> 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; > */
Hi Stefano, I think we would have a violation for the Directive 4.4 (Sections of code should not be commented out) in this case, however it’s not listed in rules.rst and I don’t remember where to look to know if it was taken into consideration for the adoption in Xen in the “tailoring” phase.
