From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.18568.1685036638036917294 for ; Thu, 25 May 2023 10:43:58 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=KUCoR825; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: osde@linux.microsoft.com) Received: from [10.137.194.171] (unknown [131.107.1.171]) by linux.microsoft.com (Postfix) with ESMTPSA id 8659020FBE87; Thu, 25 May 2023 10:43:57 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8659020FBE87 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1685036637; bh=DB7p/D9mVpdeC/vzxunl8xhFjSYoS0qR6xdxtb4+40g=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=KUCoR825ytDV01cb6hSJ21UEEzyoDRWvhvAQtA+91VyWDWT4O+QPLtr2020HREPsw Qa0tDpVVFeYdh1zY89xnJPsoyJUMWjE36uXXcZ458MSER2vMX90rbhvLOYR+W3DFc5 5P5jQlqgGhsVhD9S/sOgZoZXI7BVPOgfw7IzlCKk= Message-ID: Date: Thu, 25 May 2023 10:43:57 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [edk2-devel] GoogleTest Compatibility with MdePkg's IndustyStandard header files To: devel@edk2.groups.io, michael.d.kinney@intel.com, Pedro Falcato Cc: "Pop, Aaron" References: From: "Oliver Smith-Denny" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable 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, >=20 > Thanks for the feedback! >=20 > 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. >=20 > We will see if this resolves Aaron specific use case and if > it does we can move this to a code review. >=20 > 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. >=20 > 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__ >=20 > +// > +// 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/Ind= ustryStandard/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/Ind= ustryStandard/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; >=20 > // 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; >=20 > // Table 137 - TPMT_KEYEDHASH_SCHEME Structure >=20 >=20 > Mike >=20 >> -----Original Message----- >> From: Pedro Falcato >> Sent: Thursday, May 25, 2023 2:44 AM >> To: devel@edk2.groups.io; Kinney, Michael D >> Cc: Pop, Aaron >> Subject: Re: [edk2-devel] GoogleTest Compatibility with MdePkg's >> IndustyStandard header files >> >> On Thu, May 25, 2023 at 1:24=E2=80=AFAM Michael D Kinney >> wrote: >>> >>> That is exactly what I did. Along with pragma to disable error on macr= os >> 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 suppr= ess >> that error. >> >> FWIW, it does work here: https://godbolt.org/z/EEdh9oh53 >> >> -- >> Pedro >=20 >=20 >=20 >=20 >=20