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
next prev parent 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