[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [XEN PATCH v3] xen/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3


  • To: Federico Serafini <federico.serafini@xxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 18 Oct 2023 12:34:51 +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=+GnZTBOTiaViRjl2hI0BmZXiyG6wGJy7U9euRhgUX3Q=; b=R7IqMOgWDhe663JBmiLZV41OX1maf22gkt0aJtKBztJmTSwcnHa37q/GUn6UStL4UpGTVdLoRCsdX6iaDb71BiXAgqLqliKNyeHWMl+6nXB6W8yrka6P3Z97WW0u3UGCK1T8nJ1YaS94n245bxCS8AASvfR88J3AJSdZXR56bQFeV6msyWfDxnjLFcIzSNiwusDBcAS7N25XYueaIir+AxvF1RXpRNPTZ14SwLvLN31mt9honOMe/LkcjCrXceHngkcUlMWOwIkdfzDtYE+SRIm365qiHojcj3Zar1Ipa8pImoyGUXGvce8WRxpbifWPFHhCGqVNMHQ87INZMrJAcg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aD9uc+9HKoOKwBHbI2huLwifqBtN3c77uoMzfYaX+YxxAvELW99BJqfSZXiKpoyRL+GAmZqEsaWZYCE0jVpVvihGomXp3LABAMzPRzLedIRjhu3DpD4HYAWv+CM+93iDq2qjOyDvjVH4Hl4HD0dAEmqFyqXWET0g4kU0jbWOmmPSOQ/0O/AyfV9MENNah+b5RFEjBdtH+245bkhzlgPYGYrkqE4Xu+fX+9cdxcbaEVr+22QH51yqs8tKx42twaTR9bDykhZj/zzq8V0/o3aPKGy3EaVRHUY2kbYDRvhX4g0w7791//ANoFINPpAbFSweP8wQ2yHvbRfOba2iVdIE/g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: consulting@xxxxxxxxxxx, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Henry Wang <henry.wang@xxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 18 Oct 2023 10:35:06 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 18.10.2023 12:07, Federico Serafini wrote:
> On 16/10/23 17:26, Jan Beulich wrote:
>> On 03.10.2023 17:24, Federico Serafini wrote:
>>> --- a/xen/arch/x86/mm.c
>>> +++ b/xen/arch/x86/mm.c
>>> @@ -5901,17 +5901,17 @@ int destroy_xen_mappings(unsigned long s, unsigned 
>>> long e)
>>>    * a problem.
>>>    */
>>>   void init_or_livepatch modify_xen_mappings_lite(
>>> -    unsigned long s, unsigned long e, unsigned int _nf)
>>> +    unsigned long s, unsigned long e, unsigned int nf)
>>>   {
>>> -    unsigned long v = s, fm, nf;
>>> +    unsigned long v = s, fm, flags;
>>
>> While it looks correct, I consider this an unacceptably dangerous
>> change: What if by the time this is to be committed some new use of
>> the local "nf" appears, without resulting in fuzz while applying the
>> patch? Imo this needs doing in two steps: First nf -> flags, then
>> _nf -> nf.
>>
>> Furthermore since you alter the local variable, is there any reason
>> not to also change it to be "unsigned int", matching the function
>> argument it's set from?
> 
> If you are referring to the local variable 'flags', it is set using the
> value returned from put_pte_flags() which is an intpte_t (typedef for 
> u64). Am I missing something?

Oh, I'm sorry, I didn't look there carefully enough. Which means using
_nf and nf here was probably not a good idea in the first place, when
both are of different type and nature.

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.