From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A1291821A2 for ; Sun, 26 Feb 2017 21:10:16 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP; 26 Feb 2017 21:10:16 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,212,1484035200"; d="scan'208";a="1115945645" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 26 Feb 2017 21:10:15 -0800 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 21:10:14 -0800 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 26 Feb 2017 21:10:14 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.59]) by shsmsx102.ccr.corp.intel.com ([169.254.2.88]) with mapi id 14.03.0248.002; Mon, 27 Feb 2017 13:10:11 +0800 From: "Ni, Ruiyu" To: "Wu, Hao A" , "edk2-devel@lists.01.org" CC: "Carsey, Jaben" Thread-Topic: [PATCH v3 6/6] ShellPkg: Refine type cast for pointer subtraction Thread-Index: AQHSjxxsGjX9XqMH1kCwnFNz4CfgA6F8ULoA Date: Mon, 27 Feb 2017 05:10:10 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B8B4808@SHSMSX104.ccr.corp.intel.com> References: <1487995514-7628-1-git-send-email-hao.a.wu@intel.com> <1487995514-7628-7-git-send-email-hao.a.wu@intel.com> In-Reply-To: <1487995514-7628-7-git-send-email-hao.a.wu@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v3 6/6] ShellPkg: Refine type cast for pointer subtraction X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Feb 2017 05:10:16 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks/Ray > -----Original Message----- > From: Wu, Hao A > Sent: Saturday, February 25, 2017 12:05 PM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A ; Carsey, Jaben > ; Ni, Ruiyu > Subject: [PATCH v3 6/6] ShellPkg: Refine type cast for pointer subtractio= n >=20 > For pointer subtraction, the result is of type "ptrdiff_t". According to > the C11 standard (Committee Draft - April 12, 2011): >=20 > "When two pointers are subtracted, both shall point to elements of the > same array object, or one past the last element of the array object; the > result is the difference of the subscripts of the two array elements. The > size of the result is implementation-defined, and its type (a signed > integer type) is ptrdiff_t defined in the header. If the resul= t > is not representable in an object of that type, the behavior is > undefined." >=20 > In our codes, there are cases that the pointer subtraction is not > performed by pointers to elements of the same array object. This might > lead to potential issues, since the behavior is undefined according to C1= 1 > standard. >=20 > Also, since the size of type "ptrdiff_t" is implementation-defined. Some > static code checkers may warn that the pointer subtraction might underflo= w > first and then being cast to a bigger size. For example: >=20 > UINT8 *Ptr1, *Ptr2; > UINTN PtrDiff; > ... > PtrDiff =3D (UINTN) (Ptr1 - Ptr2); >=20 > The commit will refine the pointer subtraction expressions by casting eac= h > pointer to UINTN first and then perform the subtraction: >=20 > PtrDiff =3D (UINTN) Ptr1 - (UINTN) Ptr2; Is this the new coding rule? I think the below code is not very readable: Diff =3D ((UINTN) Ptr1 - (UINTN) Ptr2) / sizeof (STRUCTURE)) Do we have better fix? >=20 > Cc: Jaben Carsey > Cc: Ruiyu Ni > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Hao Wu > Acked-by: Laszlo Ersek > --- > ShellPkg/Application/Shell/ShellParametersProtocol.c | 6 = +++--- > ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c | 4 = ++-- > ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c > | 4 ++-- > ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c | 4 = ++-- > ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c | 4 = ++-- > 5 files changed, 11 insertions(+), 11 deletions(-) >=20 > diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c > b/ShellPkg/Application/Shell/ShellParametersProtocol.c > index 4999155..8d76fb4 100644 > --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c > +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c > @@ -5,7 +5,7 @@ > (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> Copyright (C) 2014, Red Hat, Inc. > (C) Copyright 2013 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at > @@ -1034,9 +1034,9 @@ UpdateStdInStdOutStdErr( > StrnCpyS(CommandLineCopy, StrSize(CommandLineCopy)/sizeof(CHAR16), > NewCommandLine, StrLen(NewCommandLine)); >=20 > if (FirstLocation !=3D CommandLineCopy + StrLen(CommandLineCopy) > - && ((UINTN)(FirstLocation - CommandLineCopy) < > StrLen(NewCommandLine)) > + && (((UINTN)FirstLocation - (UINTN)CommandLineCopy)/sizeof(CHAR16) > < StrLen(NewCommandLine)) > ){ > - *(NewCommandLine + (UINTN)(FirstLocation - CommandLineCopy)) =3D > CHAR_NULL; > + *(NewCommandLine + ((UINTN)FirstLocation - > (UINTN)CommandLineCopy)/sizeof(CHAR16)) =3D CHAR_NULL; > } >=20 > if (!EFI_ERROR(Status)) { > diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c > index bb2c0b9..18f15fa 100644 > --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c > +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c > @@ -2,7 +2,7 @@ > Main file for DmpStore shell Debug1 function. >=20 > (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at > @@ -364,7 +364,7 @@ AppendSingleVariableToFile ( > // > // Crc32 > // > - gBS->CalculateCrc32 (Buffer, (UINTN) (Ptr - Buffer), (UINT32 *) Ptr); > + gBS->CalculateCrc32 (Buffer, (UINTN) Ptr - (UINTN) Buffer, (UINT32 *) = Ptr); >=20 > Status =3D ShellWriteFile (FileHandle, &BufferSize, Buffer); > FreePool (Buffer); > diff --git > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView > .c > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView > .c > index 56b682a..a5b16fe 100644 > --- > a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView > .c > +++ > b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView > .c > @@ -2,7 +2,7 @@ > Tools of clarify the content of the smbios table. >=20 > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2005 - 2012, Intel Corporation. All rights reserved.
> + Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be > found at > @@ -700,7 +700,7 @@ CalculateSmbios64BitStructureCountAndLength ( > // > // Length =3D Next structure head - this structure head > // > - (*Smbios64TableLength) +=3D (UINTN) (Smbios.Raw - Raw); > + (*Smbios64TableLength) +=3D ((UINTN) Smbios.Raw - (UINTN) Raw); > if ((*Smbios64TableLength) > Smbios64EntryPoint->TableMaximumSize) { > // > // The actual table length exceeds maximum table size, > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > index d17a29d..0bcee92 100644 > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c > @@ -75,7 +75,7 @@ IsValidGuidString( > ) { > Walker++; > } else { > - if (*Walker =3D=3D L'-' && (UINTN)(Walker - PrevWalker) =3D=3D > mGuidDataLen[Index]) { > + if (*Walker =3D=3D L'-' && (((UINTN)Walker - (UINTN)PrevWalker) / = sizeof > (CHAR16)) =3D=3D mGuidDataLen[Index]) { > Walker++; > PrevWalker =3D Walker; > Index++; > @@ -85,7 +85,7 @@ IsValidGuidString( > } > } >=20 > - if ((UINTN)(Walker - PrevWalker) =3D=3D mGuidDataLen[Index]) { > + if ((((UINTN)Walker - (UINTN)PrevWalker) / sizeof (CHAR16)) =3D=3D > mGuidDataLen[Index]) { > return TRUE; > } else { > return FALSE; > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c > index dd4a740..9ae8176 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cd.c > @@ -99,10 +99,10 @@ IsCurrentFileSystem ( >=20 > Splitter2 =3D StrStr (Cwd, L":"); >=20 > - if ((UINTN) (Splitter1 - FullPath) !=3D (UINTN) (Splitter2 - Cwd)) { > + if (((UINTN) Splitter1 - (UINTN) FullPath) !=3D ((UINTN) Splitter2 - (= UINTN) > Cwd)) { > return FALSE; > } else { > - if (StrniCmp (FullPath, Cwd, (UINTN) (Splitter1 - FullPath)) =3D=3D = NULL) { > + if (StrniCmp (FullPath, Cwd, ((UINTN) Splitter1 - (UINTN) FullPath) = / sizeof > (CHAR16)) =3D=3D NULL) { > return TRUE; > } else { > return FALSE; > -- > 1.9.5.msysgit.0