Re: [PATCH v3 4/6] NetworkPkg: Refine type cast for pointer subtraction

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Wu, Hao A
> Sent: Saturday, February 25, 2017 12:05 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [PATCH v3 4/6] NetworkPkg: Refine type cast for pointer subtraction
> 
> 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;
> 
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index 745832b..1f7a4fa 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -1342,7 +1342,7 @@ HttpResponseWorker (
>      // We receive part of header of next HTTP msg.
>      //
>      if (HttpInstance->NextMsg != NULL) {
> -      HttpMsg->BodyLength = MIN ((UINTN) (HttpInstance->NextMsg -
> (CHAR8 *) Fragment.Bulk), HttpMsg->BodyLength);
> +      HttpMsg->BodyLength = MIN ((UINTN) HttpInstance->NextMsg -
> (UINTN) Fragment.Bulk, HttpMsg->BodyLength);
>        CopyMem (HttpMsg->Body, Fragment.Bulk, HttpMsg->BodyLength);
> 
>        HttpInstance->CacheLen = Fragment.Len - HttpMsg->BodyLength;
> @@ -1360,7 +1360,7 @@ HttpResponseWorker (
>          CopyMem (HttpInstance->CacheBody, Fragment.Bulk + HttpMsg-
> >BodyLength, HttpInstance->CacheLen);
>          HttpInstance->CacheOffset = 0;
> 
> -        HttpInstance->NextMsg = HttpInstance->CacheBody + (UINTN)
> (HttpInstance->NextMsg - (CHAR8 *) (Fragment.Bulk + HttpMsg-
> >BodyLength));
> +        HttpInstance->NextMsg = HttpInstance->CacheBody + ((UINTN)
> HttpInstance->NextMsg - (UINTN) (Fragment.Bulk + HttpMsg->BodyLength));
>        }
>      } else {
>        HttpMsg->BodyLength = MIN (Fragment.Len, (UINT32) HttpMsg-
> >BodyLength);
> --
> 1.9.5.msysgit.0