Re: [PATCH v3 08/12] PcAtChipsetPkg: 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: 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>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH v3 08/12] PcAtChipsetPkg: 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>
> ---
>  PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> index dcb43fa..95e0db7 100644
> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>    UART Serial Port library functions
> 
> -  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 @@ -91,19 +91,19 @@ SerialPortInitialize (
>    // Set communications format
>    //
>    OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity << 3) |
> (gStop << 2) | Data);
> -  IoWrite8 ((UINTN) (gUartBase + LCR_OFFSET), OutputData);
> +  IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> 
>    //
>    // Configure baud rate
>    //
> -  IoWrite8 ((UINTN) (gUartBase + BAUD_HIGH_OFFSET), (UINT8) (Divisor >>
> 8));
> -  IoWrite8 ((UINTN) (gUartBase + BAUD_LOW_OFFSET), (UINT8) (Divisor &
> 0xff));
> +  IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> +  IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> 
>    //
>    // Switch back to bank 0
>    //
>    OutputData = (UINT8) ((~DLAB << 7) | (gBreakSet << 6) | (gParity << 3) |
> (gStop << 2) | Data);
> -  IoWrite8 ((UINTN) (gUartBase + LCR_OFFSET), OutputData);
> +  IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> 
>    return RETURN_SUCCESS;
>  }
> @@ -470,19 +470,19 @@ SerialPortSetAttributes (
>    // Set communications format
>    //
>    OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) |
> (LcrStop << 2) | LcrData);
> -  IoWrite8 ((UINTN) (gUartBase + LCR_OFFSET), OutputData);
> +  IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> 
>    //
>    // Configure baud rate
>    //
> -  IoWrite8 ((UINTN) (gUartBase + BAUD_HIGH_OFFSET), (UINT8) (Divisor >>
> 8));
> -  IoWrite8 ((UINTN) (gUartBase + BAUD_LOW_OFFSET), (UINT8) (Divisor &
> 0xff));
> +  IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> +  IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> 
>    //
>    // Switch back to bank 0
>    //
>    OutputData = (UINT8) ((~DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) |
> (LcrStop << 2) | LcrData);
> -  IoWrite8 ((UINTN) (gUartBase + LCR_OFFSET), OutputData);
> +  IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> 
>    return RETURN_SUCCESS;
>  }
> --
> 1.9.5.msysgit.0