Re: [PATCH v2 0/6] Refine type cast for pointer subtraction

[Laszlo Ersek <lersek@redhat.com>]

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 <stddef.h> 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 <lersek@redhat.com>

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 <lersek@redhat.com>
> 
> 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(-)
>