From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 A60C481F24 for ; Tue, 24 Jan 2017 21:37:00 -0800 (PST) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3941CC0578C9; Wed, 25 Jan 2017 05:37:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-25.phx2.redhat.com [10.3.116.25]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v0P5axhx016382; Wed, 25 Jan 2017 00:37:00 -0500 To: Hao Wu , edk2-devel@ml01.01.org References: <1485322101-17128-1-git-send-email-hao.a.wu@intel.com> From: Laszlo Ersek Message-ID: <21a01af8-741a-bbd5-ba08-1e6e54f48077@redhat.com> Date: Wed, 25 Jan 2017 06:36:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <1485322101-17128-1-git-send-email-hao.a.wu@intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 25 Jan 2017 05:37:01 +0000 (UTC) Subject: Re: [PATCH v2 0/6] 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: Wed, 25 Jan 2017 05:37:00 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 01/25/17 06:28, Hao Wu wrote: > Please note that this patch is maily for feedback collection and only the > MdeModulePkg part of the series is sent out firstly. > > > V2 refines the commit messages: > For pointer subtraction, the result is of type "ptrdiff_t". According to > the C11 standard (Committee Draft - April 12, 2011): > > "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 result > is not representable in an object of that type, the behavior is > undefined." > > 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 C11 > standard. > > Also, since the size of type "ptrdiff_t" is implementation-defined. Some > static code checkers may warn that the pointer subtraction might underflow > first and then being cast to a bigger size. For example: > > UINT8 *Ptr1, *Ptr2; > UINTN PtrDiff; > ... > PtrDiff = (UINTN) (Ptr1 - Ptr2); > > The commit will refine the pointer subtraction expressions by casting each > pointer to UINTN first and then perform the subtraction: > > PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2; > This commit message update looks good to me. For all patches in the series that use this commit message (and perform the matching source code changes): Acked-by: Laszlo Ersek Module owners should of course review the patches in detail. Thanks, Laszlo > > V1: > For pointer subtraction, the result is of type "ptrdiff_t". According to > the C11 spec, ptrdiff_t is a signed integer type but its size is > implementation-defined. > > There are cases that the result of pointer subtraction is type casted to > UINTN, like: > > UINT8 *Ptr1, *Ptr2; > UINTN PtrDiff; > ... > PtrDiff = (UINTN) (Ptr1 - Ptr2); > > Some static code checkers may warn that the pointer subtraction might > overflow first and then the result is being cast to a bigger size. > > The series will cast each pointer to UINTN first and then perform the > subtraction: > > PtrDiff = (UINTN) Ptr1 - (UINTN) Ptr2; > > > Cc: Laszlo Ersek > > Hao Wu (6): > MdeModulePkg: Refine type cast for pointer subtraction > CryptoPkg: Refine type cast for pointer subtraction > IntelFrameworkModulePkg: Refine type cast for pointer subtraction > NetworkPkg: Refine type cast for pointer subtraction > SecurityPkg: Refine type cast for pointer subtraction > ShellPkg: Refine type cast for pointer subtraction > > CryptoPkg/Library/BaseCryptLib/SysCall/RuntimeMemAllocation.c | 6 +++--- > IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c | 4 ++-- > IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 4 ++-- > IntelFrameworkModulePkg/Library/GenericBdsLib/BdsConsole.c | 4 ++-- > IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c | 4 ++-- > IntelFrameworkModulePkg/Library/LegacyBootManagerLib/LegacyBm.c | 4 ++-- > IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c | 6 +++--- > MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 4 ++-- > MdeModulePkg/Include/Library/NetLib.h | 6 +++--- > MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c | 4 ++-- > MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 2 +- > MdeModulePkg/Library/FileExplorerLib/FileExplorer.c | 6 +++--- > MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c | 4 ++-- > MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 4 ++-- > MdeModulePkg/Universal/DebugPortDxe/DebugPort.c | 4 ++-- > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWrite.c | 4 ++-- > MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 4 ++-- > MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10 +++++----- > NetworkPkg/HttpDxe/HttpImpl.c | 6 +++--- > SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 18 +++++++++--------- > SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c | 10 +++++----- > SecurityPkg/Library/Tpm2CommandLib/Tpm2Help.c | 6 +++--- > SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 10 +++++----- > SecurityPkg/Tcg/TrEEDxe/MeasureBootPeCoff.c | 10 +++++----- > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 14 +++++++------- > 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 | 6 +++--- > 30 files changed, 91 insertions(+), 91 deletions(-) >