public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Fu, Siyuan" <siyuan.fu@intel.com>
Subject: Re: [PATCH v3 07/12] NetworkPkg: Refine casting expression result to bigger size
Date: Mon, 27 Feb 2017 02:21:26 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B72741629E216@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <1487999555-9764-8-git-send-email-hao.a.wu@intel.com>

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


Thanks,
Jiaxin

> -----Original Message-----
> From: Wu, Hao A
> Sent: Saturday, February 25, 2017 1:13 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 07/12] NetworkPkg: Refine casting expression result to
> bigger size
> 
> There are cases that the operands of an expression are all with rank less
> than UINT64/INT64 and the result of the expression is explicitly cast to
> UINT64/INT64 to fit the target size.
> 
> An example will be:
> UINT32 a,b;
> // a and b can be any unsigned int type with rank less than UINT64, like
> // UINT8, UINT16, etc.
> UINT64 c;
> c = (UINT64) (a + b);
> 
> Some static code checkers may warn that the expression result might
> overflow within the rank of "int" (integer promotions) and the result is
> then cast to a bigger size.
> 
> The commit refines codes by the following rules:
> 1). When the expression is possible to overflow the range of unsigned int/
> int:
> c = (UINT64)a + b;
> 
> 2). When the expression will not overflow within the rank of "int", remove
> the explicit type casts:
> c = a + b;
> 
> 3). When the expression will be cast to pointer of possible greater size:
> UINT32 a,b;
> VOID *c;
> c = (VOID *)(UINTN)(a + b); --> c = (VOID *)((UINTN)a + b);
> 
> 4). When one side of a comparison expression contains only operands with
> rank less than UINT32:
> UINT8 a;
> UINT16 b;
> UINTN c;
> if ((UINTN)(a + b) > c) {...} --> if (((UINT32)a + b) > c) {...}
> 
> For rule 4), if we remove the 'UINTN' type cast like:
> if (a + b > c) {...}
> The VS compiler will complain with warning C4018 (signed/unsigned
> mismatch, level 3 warning) due to promoting 'a + b' to type 'int'.
> 
> 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>
> ---
>  NetworkPkg/IpSecDxe/Ikev2/Payload.c   |  4 ++--
>  NetworkPkg/IpSecDxe/IpSecConfigImpl.c |  8 ++++----
>  NetworkPkg/IpSecDxe/IpSecConfigImpl.h |  4 ++--
>  NetworkPkg/Mtftp6Dxe/Mtftp6Support.c  |  4 ++--
>  NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c   | 10 +++++-----
>  5 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/NetworkPkg/IpSecDxe/Ikev2/Payload.c
> b/NetworkPkg/IpSecDxe/Ikev2/Payload.c
> index f32b3a8..237743b 100644
> --- a/NetworkPkg/IpSecDxe/Ikev2/Payload.c
> +++ b/NetworkPkg/IpSecDxe/Ikev2/Payload.c
> @@ -2,7 +2,7 @@
>    The implementation of Payloads Creation.
> 
>    (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> -  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1748,7 +1748,7 @@ Ikev2EncodeSa (
>        Transform->Header.NextPayload   =
> IKE_TRANSFORM_NEXT_PAYLOAD_MORE;
>        Transform->Header.PayloadLength = HTONS ((UINT16)TransformSize);
> 
> -      if (TransformIndex == (UINTN)(ProposalData->NumTransforms - 1)) {
> +      if (TransformIndex == ((UINT32)ProposalData->NumTransforms - 1)) {
>          Transform->Header.NextPayload =
> IKE_TRANSFORM_NEXT_PAYLOAD_NONE;
>        }
> 
> diff --git a/NetworkPkg/IpSecDxe/IpSecConfigImpl.c
> b/NetworkPkg/IpSecDxe/IpSecConfigImpl.c
> index cfee978..4a51bff 100644
> --- a/NetworkPkg/IpSecDxe/IpSecConfigImpl.c
> +++ b/NetworkPkg/IpSecDxe/IpSecConfigImpl.c
> @@ -1,7 +1,7 @@
>  /** @file
>    The implementation of IPSEC_CONFIG_PROTOCOL.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1152,7 +1152,7 @@ SetSpdEntry (
>    // Do Padding for the different Arch.
>    //
>    SpdEntrySize  = ALIGN_VARIABLE (sizeof (IPSEC_SPD_ENTRY));
> -  SpdEntrySize  = ALIGN_VARIABLE (SpdEntrySize +
> (UINTN)SIZE_OF_SPD_SELECTOR (SpdSel));
> +  SpdEntrySize  = ALIGN_VARIABLE (SpdEntrySize + SIZE_OF_SPD_SELECTOR
> (SpdSel));
>    SpdEntrySize += IpSecGetSizeOfEfiSpdData (SpdData);
> 
>    SpdEntry = AllocateZeroPool (SpdEntrySize);
> @@ -1357,7 +1357,7 @@ SetSadEntry (
>    }
> 
>    if (SaData->SpdSelector != NULL) {
> -    SadEntrySize += SadEntrySize + (UINTN)SIZE_OF_SPD_SELECTOR (SaData-
> >SpdSelector);
> +    SadEntrySize += SadEntrySize + SIZE_OF_SPD_SELECTOR (SaData-
> >SpdSelector);
>    }
>    SadEntry      = AllocateZeroPool (SadEntrySize);
> 
> @@ -1458,7 +1458,7 @@ SetSadEntry (
>        SadEntry->Data->SpdEntry = SpdEntry;
>        SadEntry->Data->SpdSelector = (EFI_IPSEC_SPD_SELECTOR *)((UINT8
> *)SadEntry +
>                                                                  SadEntrySize -
> -                                                                (UINTN)SIZE_OF_SPD_SELECTOR (SaData-
> >SpdSelector)
> +                                                                SIZE_OF_SPD_SELECTOR (SaData-
> >SpdSelector)
>                                                                  );
>        DuplicateSpdSelector (
>         (EFI_IPSEC_CONFIG_SELECTOR *) SadEntry->Data->SpdSelector,
> diff --git a/NetworkPkg/IpSecDxe/IpSecConfigImpl.h
> b/NetworkPkg/IpSecDxe/IpSecConfigImpl.h
> index 3e365da..23e6880 100644
> --- a/NetworkPkg/IpSecDxe/IpSecConfigImpl.h
> +++ b/NetworkPkg/IpSecDxe/IpSecConfigImpl.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Definitions related to IPSEC_CONFIG_PROTOCOL implementations.
> 
> -  Copyright (c) 2009 - 2011, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
> @@ -38,7 +38,7 @@
>  #define IPSECCONFIG_VARIABLE_NAME       L"IpSecConfig"
>  #define IPSECCONFIG_STATUS_NAME         L"IpSecStatus"
> 
> -#define SIZE_OF_SPD_SELECTOR(x) (UINTN) (sizeof
> (EFI_IPSEC_SPD_SELECTOR) \
> +#define SIZE_OF_SPD_SELECTOR(x) (sizeof (EFI_IPSEC_SPD_SELECTOR) \
>         + sizeof (EFI_IP_ADDRESS_INFO) * ((x)->LocalAddressCount + (x)-
> >RemoteAddressCount))
> 
>  #define FIX_REF_BUF_ADDR(addr, base)    addr = (VOID *) ((UINTN) (addr) -
> (UINTN) (base))
> diff --git a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
> b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
> index 64df901..e6b4127 100644
> --- a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
> +++ b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Mtftp6 support functions implementation.
> 
> -  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
> @@ -223,7 +223,7 @@ Mtftp6RemoveBlockNum (
>        *TotalBlock  = Num;
> 
>        if (Range->Round > 0) {
> -        *TotalBlock += Range->Bound +  MultU64x32 ((UINT64) (Range->Round
> -1), (UINT32)(Range->Bound + 1)) + 1;
> +        *TotalBlock += Range->Bound +  MultU64x32 (Range->Round - 1,
> (UINT32)(Range->Bound + 1)) + 1;
>        }
> 
>        if (Range->Start > Range->Bound) {
> diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> index e24c573..36477e9 100644
> --- a/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> +++ b/NetworkPkg/UefiPxeBcDxe/PxeBcImpl.c
> @@ -1,7 +1,7 @@
>  /** @file
>    This implementation of EFI_PXE_BASE_CODE_PROTOCOL and
> EFI_LOAD_FILE_PROTOCOL.
> 
> -  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
> 
>    This program and the accompanying materials
>    are licensed and made available under the terms and conditions of the BSD
> License
> @@ -95,8 +95,8 @@ EfiPxeBcStart (
>      //
>      // Configure block size for TFTP as a default value to handle all link layers.
>      //
> -    Private->BlockSize = (UINTN) (Private->Ip6MaxPacketSize -
> -                           PXEBC_DEFAULT_UDP_OVERHEAD_SIZE -
> PXEBC_DEFAULT_TFTP_OVERHEAD_SIZE);
> +    Private->BlockSize = Private->Ip6MaxPacketSize -
> +                           PXEBC_DEFAULT_UDP_OVERHEAD_SIZE -
> PXEBC_DEFAULT_TFTP_OVERHEAD_SIZE;
> 
>      //
>      // PXE over IPv6 starts here, initialize the fields and list header.
> @@ -148,8 +148,8 @@ EfiPxeBcStart (
>      //
>      // Configure block size for TFTP as a default value to handle all link layers.
>      //
> -    Private->BlockSize = (UINTN) (Private->Ip4MaxPacketSize -
> -                           PXEBC_DEFAULT_UDP_OVERHEAD_SIZE -
> PXEBC_DEFAULT_TFTP_OVERHEAD_SIZE);
> +    Private->BlockSize = Private->Ip4MaxPacketSize -
> +                           PXEBC_DEFAULT_UDP_OVERHEAD_SIZE -
> PXEBC_DEFAULT_TFTP_OVERHEAD_SIZE;
> 
>      //
>      // PXE over IPv4 starts here, initialize the fields.
> --
> 1.9.5.msysgit.0



  reply	other threads:[~2017-02-27  2:21 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-25  5:12 [PATCH v3 00/12] Refine casting expression result to bigger size Hao Wu
2017-02-25  5:12 ` [PATCH v3 01/12] MdePkg: " Hao Wu
2017-02-25  5:12 ` [PATCH v3 02/12] MdeModulePkg: " Hao Wu
2017-03-06  1:37   ` Tian, Feng
2017-02-25  5:12 ` [PATCH v3 03/12] FatPkg: " Hao Wu
2017-02-27  5:07   ` Ni, Ruiyu
2017-02-25  5:12 ` [PATCH v3 04/12] IntelFrameworkModulePkg: " Hao Wu
2017-02-27  7:06   ` Fan, Jeff
2017-02-25  5:12 ` [PATCH v3 05/12] IntelFsp2WrapperPkg: " Hao Wu
2017-02-25  5:51   ` Yao, Jiewen
2017-02-25  5:12 ` [PATCH v3 06/12] IntelFspWrapperPkg: " Hao Wu
2017-02-25  5:51   ` Yao, Jiewen
2017-02-25  5:12 ` [PATCH v3 07/12] NetworkPkg: " Hao Wu
2017-02-27  2:21   ` Wu, Jiaxin [this message]
2017-02-25  5:12 ` [PATCH v3 08/12] PcAtChipsetPkg: " Hao Wu
2017-02-27  7:24   ` Ni, Ruiyu
2017-02-25  5:12 ` [PATCH v3 09/12] SecurityPkg/Opal: " Hao Wu
2017-03-06  1:40   ` Dong, Eric
2017-02-25  5:12 ` [PATCH v3 10/12] ShellPkg: " Hao Wu
2017-02-27  2:22   ` Ni, Ruiyu
2017-02-27 16:38   ` Carsey, Jaben
2017-02-25  5:12 ` [PATCH v3 11/12] SourceLevelDebugPkg: " Hao Wu
2017-02-27  7:27   ` Fan, Jeff
2017-02-25  5:12 ` [PATCH v3 12/12] UefiCpuPkg: " Hao Wu
2017-02-27  7:27   ` Fan, Jeff

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B72741629E216@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox