Re: [PATCH v3 10/12] ShellPkg: Refine casting expression result to bigger size

["Carsey, Jaben" <jaben.carsey@intel.com>]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, February 24, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH v3 10/12] ShellPkg: Refine casting expression result to bigger
> size
> Importance: High
> 
> 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: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
> | 8 ++++----
> 
> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands
> Lib.c | 4 ++--
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c                             | 4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
> index 68d2443..1048ecd 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/HexEdit/BufferImage.c
> @@ -2,7 +2,7 @@
>    Defines HBufferImage - the view of the file that is visible at any point,
>    as well as the event handlers for editing the file
> 
> -  Copyright (c) 2005 - 2014, Intel Corporation. All rights reserved. <BR>
> +  Copyright (c) 2005 - 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
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -1108,15 +1108,15 @@ HBufferImageCharToHex (
>    // change the character to hex
>    //
>    if (Char >= L'0' && Char <= L'9') {
> -    return (INTN) (Char - L'0');
> +    return (Char - L'0');
>    }
> 
>    if (Char >= L'a' && Char <= L'f') {
> -    return (INTN) (Char - L'a' + 10);
> +    return (Char - L'a' + 10);
>    }
> 
>    if (Char >= L'A' && Char <= L'F') {
> -    return (INTN) (Char - L'A' + 10);
> +    return (Char - L'A' + 10);
>    }
> 
>    return -1;
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> index 6ebf002..a0e249e 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Main file for NULL named library for debug1 profile shell command
> functions.
> 
> -  Copyright (c) 2010 - 2011, 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
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -193,7 +193,7 @@ HexCharToUintn (
>      return Char - L'0';
>    }
> 
> -  return (UINTN) (10 + CharToUpper (Char) - L'A');
> +  return (10 + CharToUpper (Char) - L'A');
>  }
> 
>  /**
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index 536db3c..55e8a67 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -3,7 +3,7 @@
> 
>    (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>    Copyright 2016 Dell Inc.
> -  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2006 - 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
>    which accompanies this distribution.  The full text of the license may be
> found at
> @@ -3755,7 +3755,7 @@ InternalShellHexCharToUintn (
>      return Char - L'0';
>    }
> 
> -  return (UINTN) (10 + InternalShellCharToUpper (Char) - L'A');
> +  return (10 + InternalShellCharToUpper (Char) - L'A');
>  }
> 
>  /**
> --
> 1.9.5.msysgit.0