[PATCH v3 2/2] ArmPlatformPkg: Fix comparison of constants warning

[PATCH v3 2/2] ArmPlatformPkg: Fix comparison of constants warning

[Sami Mujawar]

The VS2017 compiler reports 'warning C6326: potential
comparison of a constant with another constant' when
a fixed PCD value is compared with a constant value.

The faulting code is as marked by '-->' below:

--> if (FixedPcdGet32 (PL011UartInteger) != 0) {
      Integer = FixedPcdGet32 (PL011UartInteger);
      Fractional = FixedPcdGet32 (PL011UartFractional);
    } else {
    ...

The macro FixedPcdGet32 (PL011UartInteger) evaluates
to a macro _PCD_VALUE_PL011UartInteger that is defined
by the build system to represent the UART Integer
value. Therefore, the VS2017 compiler reports the above
warning.

In this case the warning reported by the Visual Studio
compiler does not evaluate to an issue. However, it can
be useful to detect potential issues in other scenarios.
Other compilers may either be incapable of detecting and
reporting comparison with constant warnings or may be
good at reducing false positives. So, it is definitely
useful to keep this warning enabled.

Therefore, slight adjustments are made to the code to
stop Visual Studio from reporting this warning.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
    v3:
     - Adjusted code to fix warning instead of using a pragma     [SAMI]
    
    v2:
     - Update patch to selectively suppress comparison of         [SAMI]
       constant warning and submit as a separate series.
     - Feedback and discussion can be seen at
       https://edk2.groups.io/g/devel/topic/74289925
    
    v1:
     - Fix comparison of constant warning reported by VS2017      [SAMI]
     - Various feedbacks can be seen at:
       https://edk2.groups.io/g/devel/topic/32999801#46278

 ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index 2d3c279cce49304959953ec4a34b50e09a7d0045..d710fd3e3c8e700915d20cb4a980f51b3e078d16 100644
--- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
+++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
@@ -2,7 +2,7 @@
   Serial I/O Port library functions with no library constructor/destructor
 
   Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
-  Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -174,8 +174,8 @@ PL011UartInitializePort (
   //
 
   // If PL011 Integer value has been defined then always ignore the BAUD rate
-  if (FixedPcdGet32 (PL011UartInteger) != 0) {
-    Integer = FixedPcdGet32 (PL011UartInteger);
+  Integer = FixedPcdGet32 (PL011UartInteger);
+  if (Integer != 0) {
     Fractional = FixedPcdGet32 (PL011UartFractional);
   } else {
     // If BAUD rate is zero then replace it with the system default value
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v3 2/2] ArmPlatformPkg: Fix comparison of constants warning

[Philippe Mathieu-Daudé]

On 5/21/20 5:20 PM, Sami Mujawar wrote:
> The VS2017 compiler reports 'warning C6326: potential
> comparison of a constant with another constant' when
> a fixed PCD value is compared with a constant value.
> 
> The faulting code is as marked by '-->' below:
> 
> --> if (FixedPcdGet32 (PL011UartInteger) != 0) {
>        Integer = FixedPcdGet32 (PL011UartInteger);
>        Fractional = FixedPcdGet32 (PL011UartFractional);
>      } else {
>      ...
> 
> The macro FixedPcdGet32 (PL011UartInteger) evaluates
> to a macro _PCD_VALUE_PL011UartInteger that is defined
> by the build system to represent the UART Integer
> value. Therefore, the VS2017 compiler reports the above
> warning.
> 
> In this case the warning reported by the Visual Studio
> compiler does not evaluate to an issue. However, it can
> be useful to detect potential issues in other scenarios.
> Other compilers may either be incapable of detecting and
> reporting comparison with constant warnings or may be
> good at reducing false positives. So, it is definitely
> useful to keep this warning enabled.
> 
> Therefore, slight adjustments are made to the code to
> stop Visual Studio from reporting this warning.
> 
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> 
> Notes:
>      v3:
>       - Adjusted code to fix warning instead of using a pragma     [SAMI]
>      
>      v2:
>       - Update patch to selectively suppress comparison of         [SAMI]
>         constant warning and submit as a separate series.
>       - Feedback and discussion can be seen at
>         https://edk2.groups.io/g/devel/topic/74289925
>      
>      v1:
>       - Fix comparison of constant warning reported by VS2017      [SAMI]
>       - Various feedbacks can be seen at:
>         https://edk2.groups.io/g/devel/topic/32999801#46278
> 
>   ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> index 2d3c279cce49304959953ec4a34b50e09a7d0045..d710fd3e3c8e700915d20cb4a980f51b3e078d16 100644
> --- a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> +++ b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
> @@ -2,7 +2,7 @@
>     Serial I/O Port library functions with no library constructor/destructor
>   
>     Copyright (c) 2008 - 2010, Apple Inc. All rights reserved.<BR>
> -  Copyright (c) 2011 - 2019, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   
> @@ -174,8 +174,8 @@ PL011UartInitializePort (
>     //
>   
>     // If PL011 Integer value has been defined then always ignore the BAUD rate
> -  if (FixedPcdGet32 (PL011UartInteger) != 0) {
> -    Integer = FixedPcdGet32 (PL011UartInteger);
> +  Integer = FixedPcdGet32 (PL011UartInteger);
> +  if (Integer != 0) {

Simple, lovely :)

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

>       Fractional = FixedPcdGet32 (PL011UartFractional);
>     } else {
>       // If BAUD rate is zero then replace it with the system default value
>