[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [XEN PATCH v3 3/3] xen: fix violations of MISRA C:2012 Rule 3.1
- To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
- From: Jan Beulich <jbeulich@xxxxxxxx>
- Date: Tue, 4 Jul 2023 17:57:10 +0200
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=I7hflahPB2lhfBygcWkm3wsGcKXuow/J2FPnNPcunjI=; b=bxw7FRA737GN1b/w6B1SqrvWngreHGPwosONpyTLy6fqZ2TPimr/8VAdnCgKEoYko4km80qW+cpGH/r6WGQEoy+7fmtki8auNAxbJ9BWTtGnW7d5z1LLwm6kUUIkSizSuN5PEJA9fHenCf1V7gNe9we6sXX/G10dz2YPmxEQyVl8qDYhyjaJ1jxSBpndiGDe0f0suqus9fxAboDX3+LamCi9ckI2qoTliWIcXsmC/5XjdMpz5WAbGuXhYsKJHetctTYDzWnZ8xPpsL8Rwr+lK5GFqxkQqceTE7Teh6iDhe1c3XNxc7p7ZAB7n1U3a4P7PLU2BUbGWagZXcNf4J+uKw==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eTpCdJkrYUC5ib2k3qR8mg28jY4QG/Hjm1npOEDkaK4O746fPNo5cKOferVlxLy3YRWft0wrXN3it/hh4rg/oyQi08rtNR3QnRR4E33ngaT1tm80EE235DM2tZ0HFzI/7gAUqRLAdcixn/h8S5bmFBezqPw0lQ6eayOhRAVkzrpi4ENJ6dfE7RNsQspQI8Y8PPk3nBp9pIyXlGKG2V/dAxxWwxFbnxx4HdUqhag0kLph+jcMvhqm2vEs3xojdBPxo+req2zA97Xj7cKiyAYgd+LjhKBYnYb+lM6iuqGATaiZ4j01uTnfPaEdAZsXNVGgmwugRHAd97GB/77Pi/sZoQ==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
- Cc: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, "michal.orzel@xxxxxxx" <michal.orzel@xxxxxxx>, "xenia.ragiadakou@xxxxxxx" <xenia.ragiadakou@xxxxxxx>, "ayan.kumar.halder@xxxxxxx" <ayan.kumar.halder@xxxxxxx>, "consulting@xxxxxxxxxxx" <consulting@xxxxxxxxxxx>, "andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>, "roger.pau@xxxxxxxxxx" <roger.pau@xxxxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, "julien@xxxxxxx" <julien@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Luca Fancellu <Luca.Fancellu@xxxxxxx>
- Delivery-date: Tue, 04 Jul 2023 15:57:08 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 29.06.2023 21:20, Stefano Stabellini wrote:
> On Thu, 29 Jun 2023, Luca Fancellu wrote:
>>> On 29 Jun 2023, at 11:06, Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx> wrote:
>>> --- 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;
> */
While I'd be okay with this form, as Luca says it'll get us a different
violation, which we ought to avoid. While I was the one to suggest the
conversion to ASSERT(), having thought about it yet once more I'm now
of the opinion that _any_ transformation of this commented out piece of
code needs first understanding what was originally meant. Or
alternatively, while converting to #if form, to add a comment making
crystal clear that it's simply uncertain what was meant.
Jan
|
