> On 12 Jul 2023, at 17:17, Nicola Vetrini <[email protected]> wrote:
>
>
>
> On 12/07/23 18:02, Luca Fancellu wrote:
>>> On 12 Jul 2023, at 16:54, Nicola Vetrini <[email protected]> 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 <[email protected]>
>>> ---
>>> Following the suggestion of this message
>>> https://lore.kernel.org/xen-devel/[email protected]/
>>> 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).
Yes, I agree it does not change anything, now that I read better the message,
it is from Jan suggesting this:
#if 1
*fl -= FLI_OFFSET;
#else
if ((*fl -= FLI_OFFSET) < 0) /* FL will be always >0! */
*fl = *sl = 0;
#endif
So using /* FL will be always >0! */ instead of copying // FL will be always >0!
Anyway, I think it can be addressed on commit, whatever form the maintainer
prefers.
>
> Regards,
>
> --
> Nicola Vetrini, BSc
> Software Engineer, BUGSENG srl (https://bugseng.com)
