Re: [PATCH v3 03/12] FatPkg: Refine casting expression result to bigger size

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Hao Wu
> Sent: Saturday, February 25, 2017 1:12 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [edk2] [PATCH v3 03/12] FatPkg: 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: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  FatPkg/EnhancedFatDxe/ReadWrite.c | 4 ++--
>  FatPkg/FatPei/FatLiteAccess.c     | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/FatPkg/EnhancedFatDxe/ReadWrite.c
> b/FatPkg/EnhancedFatDxe/ReadWrite.c
> index a6e0ec4..ad3c260 100644
> --- a/FatPkg/EnhancedFatDxe/ReadWrite.c
> +++ b/FatPkg/EnhancedFatDxe/ReadWrite.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Functions that perform file read/write.
> 
> -Copyright (c) 2005 - 2015, 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
> @@ -173,7 +173,7 @@ Done:
>      // Update IFile->Position, if everything is all right
>      //
>      CurrentPos      = ODir->CurrentPos;
> -    IFile->Position = (UINT64) (CurrentPos * sizeof (FAT_DIRECTORY_ENTRY));
> +    IFile->Position = CurrentPos * sizeof (FAT_DIRECTORY_ENTRY);
>    }
> 
>    return Status;
> diff --git a/FatPkg/FatPei/FatLiteAccess.c b/FatPkg/FatPei/FatLiteAccess.c
> index 1106345..a92c5bf 100644
> --- a/FatPkg/FatPei/FatLiteAccess.c
> +++ b/FatPkg/FatPei/FatLiteAccess.c
> @@ -1,7 +1,7 @@
>  /** @file
>    FAT file system access routines for FAT recovery PEIM
> 
> -Copyright (c) 2006 - 2013, 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 @@ -393,7 +393,7 @@ FatReadFile (
>    } else {
> 
>      if ((File->Attributes & FAT_ATTR_DIRECTORY) == 0) {
> -      Size = Size < (File->FileSize - File->CurrentPos) ? Size : (UINTN) (File-
> >FileSize - File->CurrentPos);
> +      Size = Size < (File->FileSize - File->CurrentPos) ? Size :
> + (File->FileSize - File->CurrentPos);
>      }
>      //
>      // This is a normal cluster based file
> --
> 1.9.5.msysgit.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel