Hi Mike,
Thanks for looking for solutions here. This one feels like
quite a back bend, I'm imagining reading code and coming
across TpmStruct.CPLUSPLUS_OPERATOR_KEYWORD and having to
dig around quite a lot to see what goodness is going
on. Because we would have to update the C files, too, right,
depending on the test (there exist tests that want to test
static functions and so include the C file in the unit test
file). Perhaps that is an anti-pattern and googletest has
other ways of testing static functions, but in general it
feels odd to change our functional C code for a C++ unit
test framework.
But, that being said, this is an issue we face, so perhaps
it would be simpler to just rename the members to not conflict
with the C++ keywords, as previously suggested, even though
this may differ from the spec, but it would more align with
EDKII's conventions (shouty case) where the C++ keywords seem
to be lowercase. With the below patch, we would already be
changing the header file to not match with the spec, so from
a readability perspective of the header file, I think
Operator is much more readable than CPLUSPLUS_OPERATOR_KEYWORD.
Thanks,
Oliver
On 5/25/2023 10:06 AM, Michael D Kinney wrote:
Hi Pedro,
Thanks for the feedback!
Applying your pattern to edk2, we find all the usage of c++
reserved keywords and replace with a macro. We then define
that macro to either be the actual c++ reserved keyword if
build with a C compiler and rename the keyword if building
with a c++ compiler. The following patches build with VS
and GCC and should be no changes from any FW consumers of
Tpm12.h or Tpm20.h files and should support GoogleTest builds
of unit test case and code under test that will consistently
rename the reserved keyword.
We will see if this resolves Aaron specific use case and if
it does we can move this to a code review.
We can always expand the keyword set as needed in the future
if industry standard specs use C definitions that collide
with additional c++ reserved keywords.
diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
index 6597e441a6e2..b98ff33002b8 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -15,6 +15,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef __BASE_H__
#define __BASE_H__
+//
+// C++ reserved keyword defines
+//
+#if defined (__cplusplus)
+#define CPLUSPLUS_OPERATOR_KEYWORD operator_
+#define CPLUSPLUS_XOR_KEYWORD xor_
+#else
+#define CPLUSPLUS_OPERATOR_KEYWORD operator
+#define CPLUSPLUS_XOR_KEYWORD xor
+#endif
+
//
// Include processor specific binding
//
diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
b/MdePkg/Include/IndustryStandard/Tpm12.h
index 155dcc9f5f99..8551dd7c5f42 100644
--- a/MdePkg/Include/IndustryStandard/Tpm12.h
+++ b/MdePkg/Include/IndustryStandard/Tpm12.h
@@ -744,7 +744,7 @@ typedef struct tdTPM_PERMANENT_FLAGS {
BOOLEAN TPMpost;
BOOLEAN TPMpostLock;
BOOLEAN FIPS;
- BOOLEAN operator;
+ BOOLEAN CPLUSPLUS_OPERATOR_KEYWORD;
BOOLEAN enableRevokeEK;
BOOLEAN nvLocked;
BOOLEAN readSRKPub;
diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
b/MdePkg/Include/IndustryStandard/Tpm20.h
index 4440f3769f26..a96af9cfed63 100644
--- a/MdePkg/Include/IndustryStandard/Tpm20.h
+++ b/MdePkg/Include/IndustryStandard/Tpm20.h
@@ -1247,7 +1247,7 @@ typedef union {
TPMI_AES_KEY_BITS aes;
TPMI_SM4_KEY_BITS SM4;
TPM_KEY_BITS sym;
- TPMI_ALG_HASH xor;
+ TPMI_ALG_HASH CPLUSPLUS_XOR_KEYWORD;
} TPMU_SYM_KEY_BITS;
// Table 123 - TPMU_SYM_MODE Union
@@ -1320,7 +1320,7 @@ typedef struct {
// Table 136 - TPMU_SCHEME_KEYEDHASH Union
typedef union {
TPMS_SCHEME_HMAC hmac;
- TPMS_SCHEME_XOR xor;
+ TPMS_SCHEME_XOR CPLUSPLUS_XOR_KEYWORD;
} TPMU_SCHEME_KEYEDHASH;
// Table 137 - TPMT_KEYEDHASH_SCHEME Structure
Mike
-----Original Message-----
From: Pedro Falcato <pedro.falc...@gmail.com>
Sent: Thursday, May 25, 2023 2:44 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com>
Cc: Pop, Aaron <aaron...@microsoft.com>
Subject: Re: [edk2-devel] GoogleTest Compatibility with MdePkg's
IndustyStandard header files
On Thu, May 25, 2023 at 1:24 AM Michael D Kinney
<michael.d.kin...@intel.com> wrote:
That is exactly what I did. Along with pragma to disable error on macros
redefining operators.
With that change use of "operator" did not generate a warning or error and
the build completed.
Did not work for "xor", and can not find any additional pragma to suppress
that error.
FWIW, it does work here: https://godbolt.org/z/EEdh9oh53
--
Pedro
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105321): https://edk2.groups.io/g/devel/message/105321
Mute This Topic: https://groups.io/mt/99079638/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
