[Top-posting is difficult to read on technical lists; it's better to
reply inline; maybe GitHub will fix this problem after all]
On 5/20/20 1:25 PM, Sami Mujawar wrote:
Hi Philippe,
I had put a descriptive message to make people aware that the pragma effects
the next line of code only. But I agree we can abbreviate it as per your
suggestion.
Usually #pragma should be avoided at all cost, but is acceptable as last
resort aid. Since you use it in a C source file (and not an header) it
is a bit implicit that the scope is local. Sometime too verbose
documentation is harmful. Anyway I'm diverging.
However, there is an ongoing discussion about an alternative (by which we could
avoid the pragma) on another thread at
https://edk2.groups.io/g/devel/message/59948.
Good, this approach is preferable.
Regards,
Phil.
Regards,
Sami Mujawar
-----Original Message-----
From: Philippe Mathieu-Daudé <phi...@redhat.com>
Sent: 20 May 2020 11:47 AM
To: Sami Mujawar <sami.muja...@arm.com>; devel@edk2.groups.io
Cc: Alexei Fedorov <alexei.fedo...@arm.com>; Ard Biesheuvel <ard.biesheu...@arm.com>;
l...@nuviainc.com; Matteo Carlini <matteo.carl...@arm.com>; Laura Moretta
<laura.more...@arm.com>; nd <n...@arm.com>
Subject: Re: [PATCH v2 2/2] ArmPlatformPkg: Fix comparison of constants warning
Hi Sami,
On 5/18/20 2:46 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,
and disabling it case by case is a suitable option.
Therefore, disable this warning for Visual studio compilers using the
pragma suppress directive that:
'Pushes the current state of the pragma on the stack, disables the
specified warning for the next line, and then pops the warning stack
so that the pragma state is reset.'
Signed-off-by: Sami Mujawar <sami.muja...@arm.com>
---
Notes:
v2:
- Update patch to selectively suppress comparison of [SAMI]
constant warning and submit as a separate series.
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 | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
b/ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.c
index
2d3c279cce49304959953ec4a34b50e09a7d0045..3c915e1e8de22a0b0b4cc46d495a
5a6cbc784013 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,6 +174,14 @@ PL011UartInitializePort (
//
// If PL011 Integer value has been defined then always ignore the
BAUD rate
+#if defined(_MSC_EXTENSIONS)
+ // Suppress 'warning C6326' reported by Visual Studio compiler
+using
+ // the suppress pragma directive that: 'Pushes the current state of
+ // the pragma on the stack, disables the specified warning for the
+ // next line, and then pops the warning stack so that the pragma
+state
+ // is reset.'
We don't need to document how #pragma works each time we use it in the source
code...
What about a simpler comment, referring the particular Visual Studio
version:
//
// Disable 'potential comparison of a constant with another constant'
// warning with VS2017 compiler static code analysis option is enabled //
+#pragma warning(suppress:6326)
+#endif
if (FixedPcdGet32 (PL011UartInteger) != 0) {
Integer = FixedPcdGet32 (PL011UartInteger);
Fractional = FixedPcdGet32 (PL011UartFractional);
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#59957): https://edk2.groups.io/g/devel/message/59957
Mute This Topic: https://groups.io/mt/74289925/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
