[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: Mon, 16 Oct 2023 17:26:48 +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=YNFF5l0WFjxRRCJamkZRiGvDRuEwmtqhh7uCkOdO294=; b=gaQc/ojQBLgRoXTECLF2fjIXkRnt2T14UuVYVp4tkeqln1ft+vrPP/Rz6UXV/H+SlIf/YAua3ftAgbZwGO9NNgKdLJU4bAjhzDxARgUATWz+CDvuUSjAD/1cavhQ03ppP+YkwtQ5g1yLNOGWL6v+KPBWFOVU7tdpedAk0FdM5L9OUDGF57lpRfuv61AwET56dP/RwRLTK9sgGU2lU8K6Ah8cNjR+k3hbWA4uErFNVv5QYmwMlTxhnaYOOrCXxXgUGo0k1i2mc9k138V05wKQsWdzMWvFoc5Be5VgyQILRTBv/gz88vZp+Wt7DVQoqJd/hL10TU7J/Khw4+ti6NTPFw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=SRxERr2jXMhJDDtyw1JIYSsXs2Z8gHpmtKC4uwUZ8LYNgGkjKOVQkRUSBDRWtlJOu5+8dVxs812iz5VIErNekBSXRmXwzT0Z4w970y5LAEAns/5KF+OfiITJG+BDkFK1ubtoKv1ewJBsvhnt8pdmtlgpBvXyKRRfeY3iZNAMYvVRtFYpF3oNGJcrKneuXzsYYgtndpEkBcSZ4GYJzNY0fsNeNVfW67QL0hvgVG3Q4kycr7PaFBplcmWvPh2R3q+Fe8HDWVdCejIYFy2aqo4lSD9ItIWCexDKx0thh8gGcJeP0C96mXcCLO+y5EAgzb49fzVY5hhllCOaFXHCvuXh9Q==
  • 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: Mon, 16 Oct 2023 15:27:36 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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?

Yet then - can't we just delete "nf" and rename "_nf" to "nf"? The
function parameter is only used in

    nf = put_pte_flags(_nf & FLAGS_MASK);

Jan



 


Rackspace

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