[Patch v2 0/3] Address C++ keyword collisions

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [Patch v2 0/3] Address C++ keyword collisions
@ 2023-05-30 18:53 Michael D Kinney
  2023-05-30 18:53 ` [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names Michael D Kinney
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Michael D Kinney @ 2023-05-30 18:53 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Zhiguang Liu, Oliver Smith-Denny, Pedro Falcato,
	Aaron Pop

New in v2
==========
Changes from 2 patches to 3 patches to support bisect.  This
temporarily uses an anonymous union to allow use of both field
name styles.  It also allows downstream usage of these fields
to sync with the first 2 patches, update their field names,
and then sync with last patch.

PR: https://github.com/tianocore/edk2/pull/4436

===========

Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
operator and xor in C structures to support use of these
include files when building with a C++ compiler.  

Update SecurityPkg Tpm2CommandLib to use updated field names.

* Change operator -> Operator
* Change xor -> Xor

NOTE: This is a non-backwards compatible change to Tpm12.h
and Tmp20.h. And consumers of these include files that access
the "operator" or "xor" fields must be updated.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Aaron Pop <aaronpop@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>

Michael D Kinney (3):
  MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  SecurityPkg/Library/TpmCommandLib: Change xor to Xor
  MdePkg/Include/IndustryStandard: Address C++ keyword collisions

 MdePkg/Include/IndustryStandard/Tpm12.h             | 4 ++--
 MdePkg/Include/IndustryStandard/Tpm20.h             | 4 ++--
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c     | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c    | 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.40.1.windows.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
@ 2023-05-30 18:53 ` Michael D Kinney
  2023-05-31 18:17   ` Pedro Falcato
  2023-05-30 18:53 ` [Patch v2 2/3] SecurityPkg/Library/TpmCommandLib: Change xor to Xor Michael D Kinney
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Michael D Kinney @ 2023-05-30 18:53 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Zhiguang Liu, Oliver Smith-Denny, Pedro Falcato,
	Aaron Pop

Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
operator and xor in C structures to support use of these
include files when building with a C++ compiler.

This patch temporarily introduces an anonymous union to add
Operator and Xor fields to support migration from the current
field names to the new field names.

Warning 4201 is disabled for VS20xx tool chains is a temporary
change to allow the use of anonymous unions.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Aaron Pop <aaronpop@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Include/IndustryStandard/Tpm12.h | 22 ++++++++++++++++++++--
 MdePkg/Include/IndustryStandard/Tpm20.h | 25 +++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h b/MdePkg/Include/IndustryStandard/Tpm12.h
index 155dcc9f5f99..56e89d9d0835 100644
--- a/MdePkg/Include/IndustryStandard/Tpm12.h
+++ b/MdePkg/Include/IndustryStandard/Tpm12.h
@@ -9,6 +9,14 @@
 #ifndef _TPM12_H_
 #define _TPM12_H_
 
+///
+/// Temporary disable 4201 to support anonymous unions
+///
+#if defined (_MSC_EXTENSIONS)
+#pragma warning( push )
+#pragma warning ( disable : 4201 )
+#endif
+
 ///
 /// The start of TPM return codes
 ///
@@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
   BOOLEAN              TPMpost;
   BOOLEAN              TPMpostLock;
   BOOLEAN              FIPS;
-  BOOLEAN                           operator;
-  BOOLEAN                           enableRevokeEK;
+  union {
+    BOOLEAN            operator;
+    BOOLEAN            Operator;
+  };
+  BOOLEAN              enableRevokeEK;
   BOOLEAN              nvLocked;
   BOOLEAN              readSRKPub;
   BOOLEAN              tpmEstablished;
@@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
 
 #pragma pack ()
 
+///
+/// Temporary disable 4201 to support anonymous unions
+///
+#if defined (_MSC_EXTENSIONS)
+#pragma warning( pop )
+#endif
+
 #endif
diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h b/MdePkg/Include/IndustryStandard/Tpm20.h
index 4440f3769f26..a602c0d9c289 100644
--- a/MdePkg/Include/IndustryStandard/Tpm20.h
+++ b/MdePkg/Include/IndustryStandard/Tpm20.h
@@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <IndustryStandard/Tpm12.h>
 
+///
+/// Temporary disable 4201 to support anonymous unions
+///
+#if defined (_MSC_EXTENSIONS)
+#pragma warning( push )
+#pragma warning ( disable : 4201 )
+#endif
+
 #pragma pack (1)
 
 // Annex A Algorithm Constants
@@ -1247,7 +1255,10 @@ typedef union {
   TPMI_AES_KEY_BITS    aes;
   TPMI_SM4_KEY_BITS    SM4;
   TPM_KEY_BITS         sym;
-  TPMI_ALG_HASH     xor;
+  union {
+    TPMI_ALG_HASH      xor;
+    TPMI_ALG_HASH      Xor;
+  };
 } TPMU_SYM_KEY_BITS;
 
 // Table 123 - TPMU_SYM_MODE Union
@@ -1320,7 +1331,10 @@ typedef struct {
 // Table 136 - TPMU_SCHEME_KEYEDHASH Union
 typedef union {
   TPMS_SCHEME_HMAC    hmac;
-  TPMS_SCHEME_XOR  xor;
+  union {
+    TPMS_SCHEME_XOR   xor;
+    TPMS_SCHEME_XOR   Xor;
+  };
 } TPMU_SCHEME_KEYEDHASH;
 
 // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
@@ -1809,4 +1823,11 @@ typedef struct {
 #define HASH_ALG_SHA512   0x00000008
 #define HASH_ALG_SM3_256  0x00000010
 
+///
+/// Temporary disable 4201 to support anonymous unions
+///
+#if defined (_MSC_EXTENSIONS)
+#pragma warning( pop )
+#endif
+
 #endif
-- 
2.40.1.windows.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-05-30 18:53 ` [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names Michael D Kinney
@ 2023-05-31 18:17   ` Pedro Falcato
  2023-05-31 18:41     ` Michael D Kinney
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Falcato @ 2023-05-31 18:17 UTC (permalink / raw)
  To: Michael D Kinney
  Cc: devel, Liming Gao, Zhiguang Liu, Oliver Smith-Denny, Aaron Pop

On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> operator and xor in C structures to support use of these
> include files when building with a C++ compiler.
>
> This patch temporarily introduces an anonymous union to add
> Operator and Xor fields to support migration from the current
> field names to the new field names.
>
> Warning 4201 is disabled for VS20xx tool chains is a temporary
> change to allow the use of anonymous unions.
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Aaron Pop <aaronpop@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/Tpm12.h | 22 ++++++++++++++++++++--
>  MdePkg/Include/IndustryStandard/Tpm20.h | 25 +++++++++++++++++++++++--
>  2 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h b/MdePkg/Include/IndustryStandard/Tpm12.h
> index 155dcc9f5f99..56e89d9d0835 100644
> --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> @@ -9,6 +9,14 @@
>  #ifndef _TPM12_H_
>  #define _TPM12_H_
>
> +///
> +/// Temporary disable 4201 to support anonymous unions
> +///
> +#if defined (_MSC_EXTENSIONS)
> +#pragma warning( push )
> +#pragma warning ( disable : 4201 )
> +#endif
> +
>  ///
>  /// The start of TPM return codes
>  ///
> @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
>    BOOLEAN              TPMpost;
>    BOOLEAN              TPMpostLock;
>    BOOLEAN              FIPS;
> -  BOOLEAN                           operator;
> -  BOOLEAN                           enableRevokeEK;
> +  union {
> +    BOOLEAN            operator;
> +    BOOLEAN            Operator;
> +  };

Do you know if this works cleanly for the other toolchains? i.e
supported GCCs and CLANGs?
I don't *think* there's a warning for anonymous unions beyond passing
-pedantic + -std=c<something>, but it'd be good to know.
If so, we may need a pragma for this.

> +  BOOLEAN              enableRevokeEK;
>    BOOLEAN              nvLocked;
>    BOOLEAN              readSRKPub;
>    BOOLEAN              tpmEstablished;
> @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
>
>  #pragma pack ()
>
> +///
> +/// Temporary disable 4201 to support anonymous unions
> +///
> +#if defined (_MSC_EXTENSIONS)
> +#pragma warning( pop )
> +#endif
> +
>  #endif
> diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h b/MdePkg/Include/IndustryStandard/Tpm20.h
> index 4440f3769f26..a602c0d9c289 100644
> --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> @@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  #include <IndustryStandard/Tpm12.h>
>
> +///
> +/// Temporary disable 4201 to support anonymous unions
> +///
> +#if defined (_MSC_EXTENSIONS)
> +#pragma warning( push )
> +#pragma warning ( disable : 4201 )
> +#endif
> +
>  #pragma pack (1)
>
>  // Annex A Algorithm Constants
> @@ -1247,7 +1255,10 @@ typedef union {
>    TPMI_AES_KEY_BITS    aes;
>    TPMI_SM4_KEY_BITS    SM4;
>    TPM_KEY_BITS         sym;
> -  TPMI_ALG_HASH     xor;
> +  union {
> +    TPMI_ALG_HASH      xor;
> +    TPMI_ALG_HASH      Xor;
> +  };
>  } TPMU_SYM_KEY_BITS;
>
>  // Table 123 - TPMU_SYM_MODE Union
> @@ -1320,7 +1331,10 @@ typedef struct {
>  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
>  typedef union {
>    TPMS_SCHEME_HMAC    hmac;
> -  TPMS_SCHEME_XOR  xor;
> +  union {
> +    TPMS_SCHEME_XOR   xor;
> +    TPMS_SCHEME_XOR   Xor;
> +  };
>  } TPMU_SCHEME_KEYEDHASH;
>
>  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> @@ -1809,4 +1823,11 @@ typedef struct {
>  #define HASH_ALG_SHA512   0x00000008
>  #define HASH_ALG_SM3_256  0x00000010
>
> +///
> +/// Temporary disable 4201 to support anonymous unions
> +///
> +#if defined (_MSC_EXTENSIONS)
> +#pragma warning( pop )
> +#endif
> +
>  #endif
> --
> 2.40.1.windows.1
>

All in all, this looks ok to me. Although I have to say, I'm not a
huge fan of the naming style inconsistency introduced here (i.e Xor vs
hmac).
What if we made all the struct members MixedCase? Or what if we did
something like calling them xor_ and operator_?

-- 
Pedro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-05-31 18:17   ` Pedro Falcato
@ 2023-05-31 18:41     ` Michael D Kinney
  2023-06-01 15:01       ` Michael D Kinney
  0 siblings, 1 reply; 16+ messages in thread
From: Michael D Kinney @ 2023-05-31 18:41 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
	Oliver Smith-Denny, Pop, Aaron, Kinney, Michael D



> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Wednesday, May 31, 2023 11:17 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>
> Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator
> and Xor field names
> 
> On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > operator and xor in C structures to support use of these
> > include files when building with a C++ compiler.
> >
> > This patch temporarily introduces an anonymous union to add
> > Operator and Xor fields to support migration from the current
> > field names to the new field names.
> >
> > Warning 4201 is disabled for VS20xx tool chains is a temporary
> > change to allow the use of anonymous unions.
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: Aaron Pop <aaronpop@microsoft.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  MdePkg/Include/IndustryStandard/Tpm12.h | 22 ++++++++++++++++++++--
> >  MdePkg/Include/IndustryStandard/Tpm20.h | 25 +++++++++++++++++++++++--
> >  2 files changed, 43 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> b/MdePkg/Include/IndustryStandard/Tpm12.h
> > index 155dcc9f5f99..56e89d9d0835 100644
> > --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> > +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> > @@ -9,6 +9,14 @@
> >  #ifndef _TPM12_H_
> >  #define _TPM12_H_
> >
> > +///
> > +/// Temporary disable 4201 to support anonymous unions
> > +///
> > +#if defined (_MSC_EXTENSIONS)
> > +#pragma warning( push )
> > +#pragma warning ( disable : 4201 )
> > +#endif
> > +
> >  ///
> >  /// The start of TPM return codes
> >  ///
> > @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
> >    BOOLEAN              TPMpost;
> >    BOOLEAN              TPMpostLock;
> >    BOOLEAN              FIPS;
> > -  BOOLEAN                           operator;
> > -  BOOLEAN                           enableRevokeEK;
> > +  union {
> > +    BOOLEAN            operator;
> > +    BOOLEAN            Operator;
> > +  };
> 
> Do you know if this works cleanly for the other toolchains? i.e
> supported GCCs and CLANGs?
> I don't *think* there's a warning for anonymous unions beyond passing
> -pedantic + -std=c<something>, but it'd be good to know.
> If so, we may need a pragma for this.

I did not see any issues with my local testing or with EDK II CI.

> 
> > +  BOOLEAN              enableRevokeEK;
> >    BOOLEAN              nvLocked;
> >    BOOLEAN              readSRKPub;
> >    BOOLEAN              tpmEstablished;
> > @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
> >
> >  #pragma pack ()
> >
> > +///
> > +/// Temporary disable 4201 to support anonymous unions
> > +///
> > +#if defined (_MSC_EXTENSIONS)
> > +#pragma warning( pop )
> > +#endif
> > +
> >  #endif
> > diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
> b/MdePkg/Include/IndustryStandard/Tpm20.h
> > index 4440f3769f26..a602c0d9c289 100644
> > --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> > +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> > @@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  #include <IndustryStandard/Tpm12.h>
> >
> > +///
> > +/// Temporary disable 4201 to support anonymous unions
> > +///
> > +#if defined (_MSC_EXTENSIONS)
> > +#pragma warning( push )
> > +#pragma warning ( disable : 4201 )
> > +#endif
> > +
> >  #pragma pack (1)
> >
> >  // Annex A Algorithm Constants
> > @@ -1247,7 +1255,10 @@ typedef union {
> >    TPMI_AES_KEY_BITS    aes;
> >    TPMI_SM4_KEY_BITS    SM4;
> >    TPM_KEY_BITS         sym;
> > -  TPMI_ALG_HASH     xor;
> > +  union {
> > +    TPMI_ALG_HASH      xor;
> > +    TPMI_ALG_HASH      Xor;
> > +  };
> >  } TPMU_SYM_KEY_BITS;
> >
> >  // Table 123 - TPMU_SYM_MODE Union
> > @@ -1320,7 +1331,10 @@ typedef struct {
> >  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
> >  typedef union {
> >    TPMS_SCHEME_HMAC    hmac;
> > -  TPMS_SCHEME_XOR  xor;
> > +  union {
> > +    TPMS_SCHEME_XOR   xor;
> > +    TPMS_SCHEME_XOR   Xor;
> > +  };
> >  } TPMU_SCHEME_KEYEDHASH;
> >
> >  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> > @@ -1809,4 +1823,11 @@ typedef struct {
> >  #define HASH_ALG_SHA512   0x00000008
> >  #define HASH_ALG_SM3_256  0x00000010
> >
> > +///
> > +/// Temporary disable 4201 to support anonymous unions
> > +///
> > +#if defined (_MSC_EXTENSIONS)
> > +#pragma warning( pop )
> > +#endif
> > +
> >  #endif
> > --
> > 2.40.1.windows.1
> >
> 
> All in all, this looks ok to me. Although I have to say, I'm not a
> huge fan of the naming style inconsistency introduced here (i.e Xor vs
> hmac).
> What if we made all the struct members MixedCase? Or what if we did
> something like calling them xor_ and operator_?

The more we change, the greater the potential impact to downstream use of
these structures.  I prefer to do the smallest possible change to address
the c++ reserved keyword name collisions in this patch series.

I do not have strong opinion between 'xor_' vs 'Xor'.  These files are 
based on the TCG TPM Specifications that typically start field names with
lower case and camel case after that for multi-word field names.

https://trustedcomputinggroup.org/resource/tpm-library-specification/
https://trustedcomputinggroup.org/wp-content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf

> 
> --
> Pedro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-05-31 18:41     ` Michael D Kinney
@ 2023-06-01 15:01       ` Michael D Kinney
  2023-06-06 17:56         ` Michael D Kinney
  0 siblings, 1 reply; 16+ messages in thread
From: Michael D Kinney @ 2023-06-01 15:01 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
	Oliver Smith-Denny, Pop, Aaron, Kinney, Michael D

Hi Pedro,

It appears other projects have run into this same issue with the
TPM specifications and have changed the field names by appending '_'.

https://github.com/MIPS/external-tpm2/blob/d704926273cf17498c95ff0dc50b4b17e523c109/generator/structure_generator.py#L1183

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, May 31, 2023 11:42 AM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator
> and Xor field names
> 
> 
> 
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@gmail.com>
> > Sent: Wednesday, May 31, 2023 11:17 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>
> > Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator
> > and Xor field names
> >
> > On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
> > <michael.d.kinney@intel.com> wrote:
> > >
> > > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > > operator and xor in C structures to support use of these
> > > include files when building with a C++ compiler.
> > >
> > > This patch temporarily introduces an anonymous union to add
> > > Operator and Xor fields to support migration from the current
> > > field names to the new field names.
> > >
> > > Warning 4201 is disabled for VS20xx tool chains is a temporary
> > > change to allow the use of anonymous unions.
> > >
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> > > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > > Cc: Aaron Pop <aaronpop@microsoft.com>
> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > ---
> > >  MdePkg/Include/IndustryStandard/Tpm12.h | 22 ++++++++++++++++++++--
> > >  MdePkg/Include/IndustryStandard/Tpm20.h | 25 +++++++++++++++++++++++--
> > >  2 files changed, 43 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> > b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > index 155dcc9f5f99..56e89d9d0835 100644
> > > --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > @@ -9,6 +9,14 @@
> > >  #ifndef _TPM12_H_
> > >  #define _TPM12_H_
> > >
> > > +///
> > > +/// Temporary disable 4201 to support anonymous unions
> > > +///
> > > +#if defined (_MSC_EXTENSIONS)
> > > +#pragma warning( push )
> > > +#pragma warning ( disable : 4201 )
> > > +#endif
> > > +
> > >  ///
> > >  /// The start of TPM return codes
> > >  ///
> > > @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
> > >    BOOLEAN              TPMpost;
> > >    BOOLEAN              TPMpostLock;
> > >    BOOLEAN              FIPS;
> > > -  BOOLEAN                           operator;
> > > -  BOOLEAN                           enableRevokeEK;
> > > +  union {
> > > +    BOOLEAN            operator;
> > > +    BOOLEAN            Operator;
> > > +  };
> >
> > Do you know if this works cleanly for the other toolchains? i.e
> > supported GCCs and CLANGs?
> > I don't *think* there's a warning for anonymous unions beyond passing
> > -pedantic + -std=c<something>, but it'd be good to know.
> > If so, we may need a pragma for this.
> 
> I did not see any issues with my local testing or with EDK II CI.
> 
> >
> > > +  BOOLEAN              enableRevokeEK;
> > >    BOOLEAN              nvLocked;
> > >    BOOLEAN              readSRKPub;
> > >    BOOLEAN              tpmEstablished;
> > > @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
> > >
> > >  #pragma pack ()
> > >
> > > +///
> > > +/// Temporary disable 4201 to support anonymous unions
> > > +///
> > > +#if defined (_MSC_EXTENSIONS)
> > > +#pragma warning( pop )
> > > +#endif
> > > +
> > >  #endif
> > > diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
> > b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > index 4440f3769f26..a602c0d9c289 100644
> > > --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> > > +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > @@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > >  #include <IndustryStandard/Tpm12.h>
> > >
> > > +///
> > > +/// Temporary disable 4201 to support anonymous unions
> > > +///
> > > +#if defined (_MSC_EXTENSIONS)
> > > +#pragma warning( push )
> > > +#pragma warning ( disable : 4201 )
> > > +#endif
> > > +
> > >  #pragma pack (1)
> > >
> > >  // Annex A Algorithm Constants
> > > @@ -1247,7 +1255,10 @@ typedef union {
> > >    TPMI_AES_KEY_BITS    aes;
> > >    TPMI_SM4_KEY_BITS    SM4;
> > >    TPM_KEY_BITS         sym;
> > > -  TPMI_ALG_HASH     xor;
> > > +  union {
> > > +    TPMI_ALG_HASH      xor;
> > > +    TPMI_ALG_HASH      Xor;
> > > +  };
> > >  } TPMU_SYM_KEY_BITS;
> > >
> > >  // Table 123 - TPMU_SYM_MODE Union
> > > @@ -1320,7 +1331,10 @@ typedef struct {
> > >  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
> > >  typedef union {
> > >    TPMS_SCHEME_HMAC    hmac;
> > > -  TPMS_SCHEME_XOR  xor;
> > > +  union {
> > > +    TPMS_SCHEME_XOR   xor;
> > > +    TPMS_SCHEME_XOR   Xor;
> > > +  };
> > >  } TPMU_SCHEME_KEYEDHASH;
> > >
> > >  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> > > @@ -1809,4 +1823,11 @@ typedef struct {
> > >  #define HASH_ALG_SHA512   0x00000008
> > >  #define HASH_ALG_SM3_256  0x00000010
> > >
> > > +///
> > > +/// Temporary disable 4201 to support anonymous unions
> > > +///
> > > +#if defined (_MSC_EXTENSIONS)
> > > +#pragma warning( pop )
> > > +#endif
> > > +
> > >  #endif
> > > --
> > > 2.40.1.windows.1
> > >
> >
> > All in all, this looks ok to me. Although I have to say, I'm not a
> > huge fan of the naming style inconsistency introduced here (i.e Xor vs
> > hmac).
> > What if we made all the struct members MixedCase? Or what if we did
> > something like calling them xor_ and operator_?
> 
> The more we change, the greater the potential impact to downstream use of
> these structures.  I prefer to do the smallest possible change to address
> the c++ reserved keyword name collisions in this patch series.
> 
> I do not have strong opinion between 'xor_' vs 'Xor'.  These files are
> based on the TCG TPM Specifications that typically start field names with
> lower case and camel case after that for multi-word field names.
> 
> https://trustedcomputinggroup.org/resource/tpm-library-specification/
> https://trustedcomputinggroup.org/wp-
> content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf
> 
> >
> > --
> > Pedro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-06-01 15:01       ` Michael D Kinney
@ 2023-06-06 17:56         ` Michael D Kinney
  2023-06-06 21:01           ` [edk2-devel] " Pedro Falcato
  2024-02-27 23:46           ` Michael D Kinney
  0 siblings, 2 replies; 16+ messages in thread
From: Michael D Kinney @ 2023-06-06 17:56 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
	Oliver Smith-Denny, Pop, Aaron, Kinney, Michael D

Hi Pedro,

Based on this info, would you prefer we use xor_ style instead of Xor?

I can update the PR with that change.

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, June 1, 2023 8:02 AM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator
> and Xor field names
> 
> Hi Pedro,
> 
> It appears other projects have run into this same issue with the
> TPM specifications and have changed the field names by appending '_'.
> 
> https://github.com/MIPS/external-
> tpm2/blob/d704926273cf17498c95ff0dc50b4b17e523c109/generator/structure_gene
> rator.py#L1183
> 
> Mike
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, May 31, 2023 11:42 AM
> > To: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator
> > and Xor field names
> >
> >
> >
> > > -----Original Message-----
> > > From: Pedro Falcato <pedro.falcato@gmail.com>
> > > Sent: Wednesday, May 31, 2023 11:17 AM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> > > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>
> > > Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator
> > > and Xor field names
> > >
> > > On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
> > > <michael.d.kinney@intel.com> wrote:
> > > >
> > > > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > > > operator and xor in C structures to support use of these
> > > > include files when building with a C++ compiler.
> > > >
> > > > This patch temporarily introduces an anonymous union to add
> > > > Operator and Xor fields to support migration from the current
> > > > field names to the new field names.
> > > >
> > > > Warning 4201 is disabled for VS20xx tool chains is a temporary
> > > > change to allow the use of anonymous unions.
> > > >
> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > > Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> > > > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > > > Cc: Aaron Pop <aaronpop@microsoft.com>
> > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > ---
> > > >  MdePkg/Include/IndustryStandard/Tpm12.h | 22 ++++++++++++++++++++--
> > > >  MdePkg/Include/IndustryStandard/Tpm20.h | 25
> +++++++++++++++++++++++--
> > > >  2 files changed, 43 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > index 155dcc9f5f99..56e89d9d0835 100644
> > > > --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > @@ -9,6 +9,14 @@
> > > >  #ifndef _TPM12_H_
> > > >  #define _TPM12_H_
> > > >
> > > > +///
> > > > +/// Temporary disable 4201 to support anonymous unions
> > > > +///
> > > > +#if defined (_MSC_EXTENSIONS)
> > > > +#pragma warning( push )
> > > > +#pragma warning ( disable : 4201 )
> > > > +#endif
> > > > +
> > > >  ///
> > > >  /// The start of TPM return codes
> > > >  ///
> > > > @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
> > > >    BOOLEAN              TPMpost;
> > > >    BOOLEAN              TPMpostLock;
> > > >    BOOLEAN              FIPS;
> > > > -  BOOLEAN                           operator;
> > > > -  BOOLEAN                           enableRevokeEK;
> > > > +  union {
> > > > +    BOOLEAN            operator;
> > > > +    BOOLEAN            Operator;
> > > > +  };
> > >
> > > Do you know if this works cleanly for the other toolchains? i.e
> > > supported GCCs and CLANGs?
> > > I don't *think* there's a warning for anonymous unions beyond passing
> > > -pedantic + -std=c<something>, but it'd be good to know.
> > > If so, we may need a pragma for this.
> >
> > I did not see any issues with my local testing or with EDK II CI.
> >
> > >
> > > > +  BOOLEAN              enableRevokeEK;
> > > >    BOOLEAN              nvLocked;
> > > >    BOOLEAN              readSRKPub;
> > > >    BOOLEAN              tpmEstablished;
> > > > @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
> > > >
> > > >  #pragma pack ()
> > > >
> > > > +///
> > > > +/// Temporary disable 4201 to support anonymous unions
> > > > +///
> > > > +#if defined (_MSC_EXTENSIONS)
> > > > +#pragma warning( pop )
> > > > +#endif
> > > > +
> > > >  #endif
> > > > diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
> > > b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > index 4440f3769f26..a602c0d9c289 100644
> > > > --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > @@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >  #include <IndustryStandard/Tpm12.h>
> > > >
> > > > +///
> > > > +/// Temporary disable 4201 to support anonymous unions
> > > > +///
> > > > +#if defined (_MSC_EXTENSIONS)
> > > > +#pragma warning( push )
> > > > +#pragma warning ( disable : 4201 )
> > > > +#endif
> > > > +
> > > >  #pragma pack (1)
> > > >
> > > >  // Annex A Algorithm Constants
> > > > @@ -1247,7 +1255,10 @@ typedef union {
> > > >    TPMI_AES_KEY_BITS    aes;
> > > >    TPMI_SM4_KEY_BITS    SM4;
> > > >    TPM_KEY_BITS         sym;
> > > > -  TPMI_ALG_HASH     xor;
> > > > +  union {
> > > > +    TPMI_ALG_HASH      xor;
> > > > +    TPMI_ALG_HASH      Xor;
> > > > +  };
> > > >  } TPMU_SYM_KEY_BITS;
> > > >
> > > >  // Table 123 - TPMU_SYM_MODE Union
> > > > @@ -1320,7 +1331,10 @@ typedef struct {
> > > >  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
> > > >  typedef union {
> > > >    TPMS_SCHEME_HMAC    hmac;
> > > > -  TPMS_SCHEME_XOR  xor;
> > > > +  union {
> > > > +    TPMS_SCHEME_XOR   xor;
> > > > +    TPMS_SCHEME_XOR   Xor;
> > > > +  };
> > > >  } TPMU_SCHEME_KEYEDHASH;
> > > >
> > > >  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> > > > @@ -1809,4 +1823,11 @@ typedef struct {
> > > >  #define HASH_ALG_SHA512   0x00000008
> > > >  #define HASH_ALG_SM3_256  0x00000010
> > > >
> > > > +///
> > > > +/// Temporary disable 4201 to support anonymous unions
> > > > +///
> > > > +#if defined (_MSC_EXTENSIONS)
> > > > +#pragma warning( pop )
> > > > +#endif
> > > > +
> > > >  #endif
> > > > --
> > > > 2.40.1.windows.1
> > > >
> > >
> > > All in all, this looks ok to me. Although I have to say, I'm not a
> > > huge fan of the naming style inconsistency introduced here (i.e Xor vs
> > > hmac).
> > > What if we made all the struct members MixedCase? Or what if we did
> > > something like calling them xor_ and operator_?
> >
> > The more we change, the greater the potential impact to downstream use of
> > these structures.  I prefer to do the smallest possible change to address
> > the c++ reserved keyword name collisions in this patch series.
> >
> > I do not have strong opinion between 'xor_' vs 'Xor'.  These files are
> > based on the TCG TPM Specifications that typically start field names with
> > lower case and camel case after that for multi-word field names.
> >
> > https://trustedcomputinggroup.org/resource/tpm-library-specification/
> > https://trustedcomputinggroup.org/wp-
> > content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf
> >
> > >
> > > --
> > > Pedro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-06-06 17:56         ` Michael D Kinney
@ 2023-06-06 21:01           ` Pedro Falcato
  2024-02-27 23:46           ` Michael D Kinney
  1 sibling, 0 replies; 16+ messages in thread
From: Pedro Falcato @ 2023-06-06 21:01 UTC (permalink / raw)
  To: devel, michael.d.kinney
  Cc: Gao, Liming, Liu, Zhiguang, Oliver Smith-Denny, Pop, Aaron

On Tue, Jun 6, 2023 at 6:57 PM Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Pedro,
>
> Based on this info, would you prefer we use xor_ style instead of Xor?
>
> I can update the PR with that change.
>
> Mike
Hi Mike,

Sorry, I didn't know you were waiting for my input. Yes, I would
prefer xor_ style, but I'll leave it up to you.
You can add my

Reviewed-by: Pedro Falcato <pedro.falcato@gmail.com>

For the whole series, regardless of the style choice here.

-- 
Pedro

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2023-06-06 17:56         ` Michael D Kinney
  2023-06-06 21:01           ` [edk2-devel] " Pedro Falcato
@ 2024-02-27 23:46           ` Michael D Kinney
  2024-02-27 23:55             ` Pedro Falcato
  1 sibling, 1 reply; 16+ messages in thread
From: Michael D Kinney @ 2024-02-27 23:46 UTC (permalink / raw)
  To: Pedro Falcato
  Cc: devel@edk2.groups.io, Gao, Liming, Liu, Zhiguang,
	Oliver Smith-Denny, Pop, Aaron, Kinney, Michael D

Hi Pedro,

Looks like this series got set aside waiting for some additional feedback.

I would like to see this C++ Keyword issue resolved.

Thanks,

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, June 6, 2023 10:57 AM
> To: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator and Xor field names
> 
> Hi Pedro,
> 
> Based on this info, would you prefer we use xor_ style instead of Xor?
> 
> I can update the PR with that change.
> 
> Mike
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Thursday, June 1, 2023 8:02 AM
> > To: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
> Liu,
> > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>;
> Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator
> > and Xor field names
> >
> > Hi Pedro,
> >
> > It appears other projects have run into this same issue with the
> > TPM specifications and have changed the field names by appending '_'.
> >
> > https://github.com/MIPS/external-
> >
> tpm2/blob/d704926273cf17498c95ff0dc50b4b17e523c109/generator/structure_
> gene
> > rator.py#L1183
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Wednesday, May 31, 2023 11:42 AM
> > > To: Pedro Falcato <pedro.falcato@gmail.com>
> > > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
> Liu,
> > > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>;
> Kinney,
> > > Michael D <michael.d.kinney@intel.com>
> > > Subject: RE: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> Operator
> > > and Xor field names
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Pedro Falcato <pedro.falcato@gmail.com>
> > > > Sent: Wednesday, May 31, 2023 11:17 AM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Cc: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
> Liu,
> > > > Zhiguang <zhiguang.liu@intel.com>; Oliver Smith-Denny
> > > > <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>
> > > > Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add
> > Operator
> > > > and Xor field names
> > > >
> > > > On Tue, May 30, 2023 at 7:53 PM Michael D Kinney
> > > > <michael.d.kinney@intel.com> wrote:
> > > > >
> > > > > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > > > > operator and xor in C structures to support use of these
> > > > > include files when building with a C++ compiler.
> > > > >
> > > > > This patch temporarily introduces an anonymous union to add
> > > > > Operator and Xor fields to support migration from the current
> > > > > field names to the new field names.
> > > > >
> > > > > Warning 4201 is disabled for VS20xx tool chains is a temporary
> > > > > change to allow the use of anonymous unions.
> > > > >
> > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > > > Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> > > > > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > > > > Cc: Aaron Pop <aaronpop@microsoft.com>
> > > > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > ---
> > > > >  MdePkg/Include/IndustryStandard/Tpm12.h | 22
> ++++++++++++++++++++--
> > > > >  MdePkg/Include/IndustryStandard/Tpm20.h | 25
> > +++++++++++++++++++++++--
> > > > >  2 files changed, 43 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > index 155dcc9f5f99..56e89d9d0835 100644
> > > > > --- a/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > +++ b/MdePkg/Include/IndustryStandard/Tpm12.h
> > > > > @@ -9,6 +9,14 @@
> > > > >  #ifndef _TPM12_H_
> > > > >  #define _TPM12_H_
> > > > >
> > > > > +///
> > > > > +/// Temporary disable 4201 to support anonymous unions
> > > > > +///
> > > > > +#if defined (_MSC_EXTENSIONS)
> > > > > +#pragma warning( push )
> > > > > +#pragma warning ( disable : 4201 )
> > > > > +#endif
> > > > > +
> > > > >  ///
> > > > >  /// The start of TPM return codes
> > > > >  ///
> > > > > @@ -744,8 +752,11 @@ typedef struct tdTPM_PERMANENT_FLAGS {
> > > > >    BOOLEAN              TPMpost;
> > > > >    BOOLEAN              TPMpostLock;
> > > > >    BOOLEAN              FIPS;
> > > > > -  BOOLEAN                           operator;
> > > > > -  BOOLEAN                           enableRevokeEK;
> > > > > +  union {
> > > > > +    BOOLEAN            operator;
> > > > > +    BOOLEAN            Operator;
> > > > > +  };
> > > >
> > > > Do you know if this works cleanly for the other toolchains? i.e
> > > > supported GCCs and CLANGs?
> > > > I don't *think* there's a warning for anonymous unions beyond
> passing
> > > > -pedantic + -std=c<something>, but it'd be good to know.
> > > > If so, we may need a pragma for this.
> > >
> > > I did not see any issues with my local testing or with EDK II CI.
> > >
> > > >
> > > > > +  BOOLEAN              enableRevokeEK;
> > > > >    BOOLEAN              nvLocked;
> > > > >    BOOLEAN              readSRKPub;
> > > > >    BOOLEAN              tpmEstablished;
> > > > > @@ -2162,4 +2173,11 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
> > > > >
> > > > >  #pragma pack ()
> > > > >
> > > > > +///
> > > > > +/// Temporary disable 4201 to support anonymous unions
> > > > > +///
> > > > > +#if defined (_MSC_EXTENSIONS)
> > > > > +#pragma warning( pop )
> > > > > +#endif
> > > > > +
> > > > >  #endif
> > > > > diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > > index 4440f3769f26..a602c0d9c289 100644
> > > > > --- a/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > > +++ b/MdePkg/Include/IndustryStandard/Tpm20.h
> > > > > @@ -15,6 +15,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > > >
> > > > >  #include <IndustryStandard/Tpm12.h>
> > > > >
> > > > > +///
> > > > > +/// Temporary disable 4201 to support anonymous unions
> > > > > +///
> > > > > +#if defined (_MSC_EXTENSIONS)
> > > > > +#pragma warning( push )
> > > > > +#pragma warning ( disable : 4201 )
> > > > > +#endif
> > > > > +
> > > > >  #pragma pack (1)
> > > > >
> > > > >  // Annex A Algorithm Constants
> > > > > @@ -1247,7 +1255,10 @@ typedef union {
> > > > >    TPMI_AES_KEY_BITS    aes;
> > > > >    TPMI_SM4_KEY_BITS    SM4;
> > > > >    TPM_KEY_BITS         sym;
> > > > > -  TPMI_ALG_HASH     xor;
> > > > > +  union {
> > > > > +    TPMI_ALG_HASH      xor;
> > > > > +    TPMI_ALG_HASH      Xor;
> > > > > +  };
> > > > >  } TPMU_SYM_KEY_BITS;
> > > > >
> > > > >  // Table 123 - TPMU_SYM_MODE Union
> > > > > @@ -1320,7 +1331,10 @@ typedef struct {
> > > > >  // Table 136 - TPMU_SCHEME_KEYEDHASH Union
> > > > >  typedef union {
> > > > >    TPMS_SCHEME_HMAC    hmac;
> > > > > -  TPMS_SCHEME_XOR  xor;
> > > > > +  union {
> > > > > +    TPMS_SCHEME_XOR   xor;
> > > > > +    TPMS_SCHEME_XOR   Xor;
> > > > > +  };
> > > > >  } TPMU_SCHEME_KEYEDHASH;
> > > > >
> > > > >  // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
> > > > > @@ -1809,4 +1823,11 @@ typedef struct {
> > > > >  #define HASH_ALG_SHA512   0x00000008
> > > > >  #define HASH_ALG_SM3_256  0x00000010
> > > > >
> > > > > +///
> > > > > +/// Temporary disable 4201 to support anonymous unions
> > > > > +///
> > > > > +#if defined (_MSC_EXTENSIONS)
> > > > > +#pragma warning( pop )
> > > > > +#endif
> > > > > +
> > > > >  #endif
> > > > > --
> > > > > 2.40.1.windows.1
> > > > >
> > > >
> > > > All in all, this looks ok to me. Although I have to say, I'm not
> a
> > > > huge fan of the naming style inconsistency introduced here (i.e
> Xor vs
> > > > hmac).
> > > > What if we made all the struct members MixedCase? Or what if we
> did
> > > > something like calling them xor_ and operator_?
> > >
> > > The more we change, the greater the potential impact to downstream
> use of
> > > these structures.  I prefer to do the smallest possible change to
> address
> > > the c++ reserved keyword name collisions in this patch series.
> > >
> > > I do not have strong opinion between 'xor_' vs 'Xor'.  These files
> are
> > > based on the TCG TPM Specifications that typically start field
> names with
> > > lower case and camel case after that for multi-word field names.
> > >
> > > https://trustedcomputinggroup.org/resource/tpm-library-
> specification/
> > > https://trustedcomputinggroup.org/wp-
> > > content/uploads/TCG_TPM2_r1p59_Part2_Structures_pub.pdf
> > >
> > > >
> > > > --
> > > > Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116064): https://edk2.groups.io/g/devel/message/116064
Mute This Topic: https://groups.io/mt/99226543/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2024-02-27 23:46           ` Michael D Kinney
@ 2024-02-27 23:55             ` Pedro Falcato
  2024-02-28  0:33               ` Michael D Kinney
  0 siblings, 1 reply; 16+ messages in thread
From: Pedro Falcato @ 2024-02-27 23:55 UTC (permalink / raw)
  To: devel, michael.d.kinney
  Cc: Gao, Liming, Liu, Zhiguang, Oliver Smith-Denny, Pop, Aaron

On Tue, Feb 27, 2024 at 11:46 PM Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
> Hi Pedro,
>
> Looks like this series got set aside waiting for some additional feedback.

Hi,

This is odd, I did reply back in June
(https://edk2.groups.io/g/devel/message/105817). You probably missed
the email or maybe for some unknown reason it never reached you.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116065): https://edk2.groups.io/g/devel/message/116065
Mute This Topic: https://groups.io/mt/99226543/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names
  2024-02-27 23:55             ` Pedro Falcato
@ 2024-02-28  0:33               ` Michael D Kinney
  0 siblings, 0 replies; 16+ messages in thread
From: Michael D Kinney @ 2024-02-28  0:33 UTC (permalink / raw)
  To: Pedro Falcato, devel@edk2.groups.io
  Cc: Gao, Liming, Liu, Zhiguang, Oliver Smith-Denny, Pop, Aaron,
	Kinney, Michael D

Thanks.  Not sure how I missed that.

I will refresh the series/PR and make sure there are no issues.

Mike

> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Tuesday, February 27, 2024 3:56 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Oliver Smith-Denny
> <osde@linux.microsoft.com>; Pop, Aaron <aaronpop@microsoft.com>
> Subject: Re: [edk2-devel] [Patch v2 1/3]
> MdePkg/Include/IndustryStandard: Add Operator and Xor field names
> 
> On Tue, Feb 27, 2024 at 11:46 PM Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> > Hi Pedro,
> >
> > Looks like this series got set aside waiting for some additional
> feedback.
> 
> Hi,
> 
> This is odd, I did reply back in June
> (https://edk2.groups.io/g/devel/message/105817). You probably missed
> the email or maybe for some unknown reason it never reached you.
> 
> --
> Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116067): https://edk2.groups.io/g/devel/message/116067
Mute This Topic: https://groups.io/mt/99226543/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Patch v2 2/3] SecurityPkg/Library/TpmCommandLib: Change xor to Xor
  2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
  2023-05-30 18:53 ` [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names Michael D Kinney
@ 2023-05-30 18:53 ` Michael D Kinney
  2023-05-31  2:38   ` [edk2-devel] [PATCH] ShellPkg\SmbiosView: SmBiosView does not print correct Slot ID information asperber
  2023-05-30 18:53 ` [Patch v2 3/3] MdePkg/Include/IndustryStandard: Address C++ keyword collisions Michael D Kinney
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Michael D Kinney @ 2023-05-30 18:53 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Oliver Smith-Denny, Pedro Falcato,
	Aaron Pop

Change xor to Xor to avoid C++ reserved work name collisions
when building with C++ compilers.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Aaron Pop <aaronpop@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c     | 6 +++---
 SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c    | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
index f0e6019a47be..f8c781a445a1 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
@@ -734,9 +734,9 @@ Tpm2TestParms (
           Buffer += sizeof (UINT16);
           break;
         case TPM_ALG_XOR:
-          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.keyedHashDetail.scheme.details.xor.hashAlg));
+          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.keyedHashDetail.scheme.details.Xor.hashAlg));
           Buffer += sizeof (UINT16);
-          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.keyedHashDetail.scheme.details.xor.kdf));
+          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.keyedHashDetail.scheme.details.Xor.kdf));
           Buffer += sizeof (UINT16);
           break;
         default:
@@ -761,7 +761,7 @@ Tpm2TestParms (
           Buffer += sizeof (UINT16);
           break;
         case TPM_ALG_XOR:
-          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.symDetail.keyBits.xor));
+          WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Parameters->parameters.symDetail.keyBits.Xor));
           Buffer += sizeof (UINT16);
           break;
         case TPM_ALG_NULL:
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
index 335957d6cedc..578a61b20339 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c
@@ -169,9 +169,9 @@ Tpm2ReadPublic (
           Buffer                                                                      += sizeof (UINT16);
           break;
         case TPM_ALG_XOR:
-          OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.xor.hashAlg = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
+          OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.Xor.hashAlg = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
           Buffer                                                                     += sizeof (UINT16);
-          OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.xor.kdf     = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
+          OutPublic->publicArea.parameters.keyedHashDetail.scheme.details.Xor.kdf     = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
           Buffer                                                                     += sizeof (UINT16);
           break;
         default:
@@ -195,7 +195,7 @@ Tpm2ReadPublic (
           Buffer                                                += sizeof (UINT16);
           break;
         case TPM_ALG_XOR:
-          OutPublic->publicArea.parameters.symDetail.keyBits.xor = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
+          OutPublic->publicArea.parameters.symDetail.keyBits.Xor = SwapBytes16 (ReadUnaligned16 ((UINT16 *)Buffer));
           Buffer                                                += sizeof (UINT16);
           break;
         case TPM_ALG_NULL:
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
index 7f247da301fe..e17318b6e6fb 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c
@@ -119,7 +119,7 @@ Tpm2StartAuthSession (
       Buffer += sizeof (UINT16);
       break;
     case TPM_ALG_XOR:
-      WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Symmetric->keyBits.xor));
+      WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (Symmetric->keyBits.Xor));
       Buffer += sizeof (UINT16);
       break;
     default:
-- 
2.40.1.windows.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH] ShellPkg\SmbiosView: SmBiosView does not print correct Slot ID information
  2023-05-30 18:53 ` [Patch v2 2/3] SecurityPkg/Library/TpmCommandLib: Change xor to Xor Michael D Kinney
@ 2023-05-31  2:38   ` asperber
  0 siblings, 0 replies; 16+ messages in thread
From: asperber @ 2023-05-31  2:38 UTC (permalink / raw)
  To: devel@edk2.groups.io, Demeter, Miki, Gao, Liming

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4469

SlotType related check only supports PcieGen3 checks within
DisplaySystemSlotId.
SmbiosView will print:
Slot Id: undefined Slot Id
Updating this check to Gen6andBeyond which fixes the problem:
Slot Id: the value present in the Slot Number field of the PCI Interrupt
Routing table entry that is associated with this slot is: <currrent
index>

Signed-off-by: Adrian Sperber <asperber@amperecomputing.com>
---
 .../Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
index a14b79904d..b109934f58 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
@@ -2998,7 +2998,7 @@ DisplaySystemSlotId (
       break;

     default:
-      if (((SlotType >= 0x0E) && (SlotType <= 0x12)) || ((SlotType >= 0xA6) && (SlotType <= 0xB6))) {
+      if (((SlotType >= 0x0E) && (SlotType <= 0x12)) || ((SlotType >= 0xA6) && (SlotType <= SlotTypePCIExpressGen6andBeyond))) {
         ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_VALUE_PRESENT), gShellDebug1HiiHandle, SlotId);
       } else {
         ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_SMBIOSVIEW_PRINTINFO_UNDEFINED_SLOT_ID), gShellDebug1HiiHandle);
--
2.38.1.windows.1

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to Ampere Computing or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. Any unauthorized review, copying, or distribution of this email (or any attachments thereto) is strictly prohibited. If you are not the intended recipient, please contact the sender immediately and permanently delete the original and any copies of this email and any attachments thereto.

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Patch v2 3/3] MdePkg/Include/IndustryStandard: Address C++ keyword collisions
  2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
  2023-05-30 18:53 ` [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names Michael D Kinney
  2023-05-30 18:53 ` [Patch v2 2/3] SecurityPkg/Library/TpmCommandLib: Change xor to Xor Michael D Kinney
@ 2023-05-30 18:53 ` Michael D Kinney
  2023-05-31  0:05 ` [edk2-devel] [Patch v2 0/3] " Yao, Jiewen
  2023-05-31 17:46 ` Oliver Smith-Denny
  4 siblings, 0 replies; 16+ messages in thread
From: Michael D Kinney @ 2023-05-30 18:53 UTC (permalink / raw)
  To: devel
  Cc: Liming Gao, Zhiguang Liu, Oliver Smith-Denny, Pedro Falcato,
	Aaron Pop

Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
operator and xor in C structures to support use of these
include files when building with a C++ compiler.

This patch removes the temporary use of anonymous unions and
warning 4201 disable for VS20xx tool chains to complete the
following field name changes:

* operator -> Operator
* xor -> Xor

NOTE: This is a non-backwards compatible change to Tpm12.h
and Tmp20.h. And consumers of these include files that access
the "operator" or "xor" fields must be updated.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Cc: Aaron Pop <aaronpop@microsoft.com>
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 MdePkg/Include/IndustryStandard/Tpm12.h | 20 +-------------------
 MdePkg/Include/IndustryStandard/Tpm20.h | 25 ++-----------------------
 2 files changed, 3 insertions(+), 42 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Tpm12.h b/MdePkg/Include/IndustryStandard/Tpm12.h
index 56e89d9d0835..147c0863fffd 100644
--- a/MdePkg/Include/IndustryStandard/Tpm12.h
+++ b/MdePkg/Include/IndustryStandard/Tpm12.h
@@ -9,14 +9,6 @@
 #ifndef _TPM12_H_
 #define _TPM12_H_
 
-///
-/// Temporary disable 4201 to support anonymous unions
-///
-#if defined (_MSC_EXTENSIONS)
-#pragma warning( push )
-#pragma warning ( disable : 4201 )
-#endif
-
 ///
 /// The start of TPM return codes
 ///
@@ -752,10 +744,7 @@ typedef struct tdTPM_PERMANENT_FLAGS {
   BOOLEAN              TPMpost;
   BOOLEAN              TPMpostLock;
   BOOLEAN              FIPS;
-  union {
-    BOOLEAN            operator;
-    BOOLEAN            Operator;
-  };
+  BOOLEAN              Operator;
   BOOLEAN              enableRevokeEK;
   BOOLEAN              nvLocked;
   BOOLEAN              readSRKPub;
@@ -2173,11 +2162,4 @@ typedef struct tdTPM_RSP_COMMAND_HDR {
 
 #pragma pack ()
 
-///
-/// Temporary disable 4201 to support anonymous unions
-///
-#if defined (_MSC_EXTENSIONS)
-#pragma warning( pop )
-#endif
-
 #endif
diff --git a/MdePkg/Include/IndustryStandard/Tpm20.h b/MdePkg/Include/IndustryStandard/Tpm20.h
index a602c0d9c289..c827af13efd0 100644
--- a/MdePkg/Include/IndustryStandard/Tpm20.h
+++ b/MdePkg/Include/IndustryStandard/Tpm20.h
@@ -15,14 +15,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <IndustryStandard/Tpm12.h>
 
-///
-/// Temporary disable 4201 to support anonymous unions
-///
-#if defined (_MSC_EXTENSIONS)
-#pragma warning( push )
-#pragma warning ( disable : 4201 )
-#endif
-
 #pragma pack (1)
 
 // Annex A Algorithm Constants
@@ -1255,10 +1247,7 @@ typedef union {
   TPMI_AES_KEY_BITS    aes;
   TPMI_SM4_KEY_BITS    SM4;
   TPM_KEY_BITS         sym;
-  union {
-    TPMI_ALG_HASH      xor;
-    TPMI_ALG_HASH      Xor;
-  };
+  TPMI_ALG_HASH        Xor;
 } TPMU_SYM_KEY_BITS;
 
 // Table 123 - TPMU_SYM_MODE Union
@@ -1331,10 +1320,7 @@ typedef struct {
 // Table 136 - TPMU_SCHEME_KEYEDHASH Union
 typedef union {
   TPMS_SCHEME_HMAC    hmac;
-  union {
-    TPMS_SCHEME_XOR   xor;
-    TPMS_SCHEME_XOR   Xor;
-  };
+  TPMS_SCHEME_XOR     Xor;
 } TPMU_SCHEME_KEYEDHASH;
 
 // Table 137 - TPMT_KEYEDHASH_SCHEME Structure
@@ -1823,11 +1809,4 @@ typedef struct {
 #define HASH_ALG_SHA512   0x00000008
 #define HASH_ALG_SM3_256  0x00000010
 
-///
-/// Temporary disable 4201 to support anonymous unions
-///
-#if defined (_MSC_EXTENSIONS)
-#pragma warning( pop )
-#endif
-
 #endif
-- 
2.40.1.windows.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [edk2-devel] [Patch v2 0/3] Address C++ keyword collisions
  2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
                   ` (2 preceding siblings ...)
  2023-05-30 18:53 ` [Patch v2 3/3] MdePkg/Include/IndustryStandard: Address C++ keyword collisions Michael D Kinney
@ 2023-05-31  0:05 ` Yao, Jiewen
  2023-05-31 17:46 ` Oliver Smith-Denny
  4 siblings, 0 replies; 16+ messages in thread
From: Yao, Jiewen @ 2023-05-31  0:05 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Gao, Liming, Liu, Zhiguang, Oliver Smith-Denny, Pedro Falcato,
	Pop, Aaron

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

Hi Mike
Do you think if we can enhance CI to catch such keyword?
I don't mean to need it in this patch, but something we could consider later.

Thank you
Yao, Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
> Kinney
> Sent: Wednesday, May 31, 2023 2:53 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>; Oliver Smith-Denny <osde@linux.microsoft.com>;
> Pedro Falcato <pedro.falcato@gmail.com>; Pop, Aaron
> <aaronpop@microsoft.com>
> Subject: [edk2-devel] [Patch v2 0/3] Address C++ keyword collisions
> 
> New in v2
> ==========
> Changes from 2 patches to 3 patches to support bisect.  This
> temporarily uses an anonymous union to allow use of both field
> name styles.  It also allows downstream usage of these fields
> to sync with the first 2 patches, update their field names,
> and then sync with last patch.
> 
> PR: https://github.com/tianocore/edk2/pull/4436
> 
> ===========
> 
> Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> operator and xor in C structures to support use of these
> include files when building with a C++ compiler.
> 
> Update SecurityPkg Tpm2CommandLib to use updated field names.
> 
> * Change operator -> Operator
> * Change xor -> Xor
> 
> NOTE: This is a non-backwards compatible change to Tpm12.h
> and Tmp20.h. And consumers of these include files that access
> the "operator" or "xor" fields must be updated.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Aaron Pop <aaronpop@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> Michael D Kinney (3):
>   MdePkg/Include/IndustryStandard: Add Operator and Xor field names
>   SecurityPkg/Library/TpmCommandLib: Change xor to Xor
>   MdePkg/Include/IndustryStandard: Address C++ keyword collisions
> 
>  MdePkg/Include/IndustryStandard/Tpm12.h             | 4 ++--
>  MdePkg/Include/IndustryStandard/Tpm20.h             | 4 ++--
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c     | 6 +++---
>  SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c    | 2 +-
>  5 files changed, 11 insertions(+), 11 deletions(-)
> 
> --
> 2.40.1.windows.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Patch v2 0/3] Address C++ keyword collisions
  2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
                   ` (3 preceding siblings ...)
  2023-05-31  0:05 ` [edk2-devel] [Patch v2 0/3] " Yao, Jiewen
@ 2023-05-31 17:46 ` Oliver Smith-Denny
  2023-06-01  7:30   ` 回复: [edk2-devel] " gaoliming
  4 siblings, 1 reply; 16+ messages in thread
From: Oliver Smith-Denny @ 2023-05-31 17:46 UTC (permalink / raw)
  To: Michael D Kinney, devel
  Cc: Liming Gao, Zhiguang Liu, Pedro Falcato, Aaron Pop

For the patchset:

Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>

Thanks!

On 5/30/2023 11:53 AM, Michael D Kinney wrote:
> New in v2
> ==========
> Changes from 2 patches to 3 patches to support bisect.  This
> temporarily uses an anonymous union to allow use of both field
> name styles.  It also allows downstream usage of these fields
> to sync with the first 2 patches, update their field names,
> and then sync with last patch.
> 
> PR: https://github.com/tianocore/edk2/pull/4436
> 
> ===========
> 
> Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> operator and xor in C structures to support use of these
> include files when building with a C++ compiler.
> 
> Update SecurityPkg Tpm2CommandLib to use updated field names.
> 
> * Change operator -> Operator
> * Change xor -> Xor
> 
> NOTE: This is a non-backwards compatible change to Tpm12.h
> and Tmp20.h. And consumers of these include files that access
> the "operator" or "xor" fields must be updated.
> 
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> Cc: Pedro Falcato <pedro.falcato@gmail.com>
> Cc: Aaron Pop <aaronpop@microsoft.com>
> Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> 
> Michael D Kinney (3):
>    MdePkg/Include/IndustryStandard: Add Operator and Xor field names
>    SecurityPkg/Library/TpmCommandLib: Change xor to Xor
>    MdePkg/Include/IndustryStandard: Address C++ keyword collisions
> 
>   MdePkg/Include/IndustryStandard/Tpm12.h             | 4 ++--
>   MdePkg/Include/IndustryStandard/Tpm20.h             | 4 ++--
>   SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
>   SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c     | 6 +++---
>   SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c    | 2 +-
>   5 files changed, 11 insertions(+), 11 deletions(-)
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* 回复: [edk2-devel] [Patch v2 0/3] Address C++ keyword collisions
  2023-05-31 17:46 ` Oliver Smith-Denny
@ 2023-06-01  7:30   ` gaoliming
  0 siblings, 0 replies; 16+ messages in thread
From: gaoliming @ 2023-06-01  7:30 UTC (permalink / raw)
  To: devel, osde, 'Michael D Kinney'
  Cc: 'Zhiguang Liu', 'Pedro Falcato',
	'Aaron Pop'

For the patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Oliver
> Smith-Denny
> 发送时间: 2023年6月1日 1:46
> 收件人: Michael D Kinney <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> 抄送: Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu
> <zhiguang.liu@intel.com>; Pedro Falcato <pedro.falcato@gmail.com>; Aaron
> Pop <aaronpop@microsoft.com>
> 主题: Re: [edk2-devel] [Patch v2 0/3] Address C++ keyword collisions
> 
> For the patchset:
> 
> Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> 
> Thanks!
> 
> On 5/30/2023 11:53 AM, Michael D Kinney wrote:
> > New in v2
> > ==========
> > Changes from 2 patches to 3 patches to support bisect.  This
> > temporarily uses an anonymous union to allow use of both field
> > name styles.  It also allows downstream usage of these fields
> > to sync with the first 2 patches, update their field names,
> > and then sync with last patch.
> >
> > PR: https://github.com/tianocore/edk2/pull/4436
> >
> > ===========
> >
> > Update Tpm12.h and Tpm20.h and not use c++ reserved keywords
> > operator and xor in C structures to support use of these
> > include files when building with a C++ compiler.
> >
> > Update SecurityPkg Tpm2CommandLib to use updated field names.
> >
> > * Change operator -> Operator
> > * Change xor -> Xor
> >
> > NOTE: This is a non-backwards compatible change to Tpm12.h
> > and Tmp20.h. And consumers of these include files that access
> > the "operator" or "xor" fields must be updated.
> >
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Cc: Aaron Pop <aaronpop@microsoft.com>
> > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> >
> > Michael D Kinney (3):
> >    MdePkg/Include/IndustryStandard: Add Operator and Xor field names
> >    SecurityPkg/Library/TpmCommandLib: Change xor to Xor
> >    MdePkg/Include/IndustryStandard: Address C++ keyword collisions
> >
> >   MdePkg/Include/IndustryStandard/Tpm12.h             | 4 ++--
> >   MdePkg/Include/IndustryStandard/Tpm20.h             | 4 ++--
> >   SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c | 6 +++---
> >   SecurityPkg/Library/Tpm2CommandLib/Tpm2Object.c     | 6 +++---
> >   SecurityPkg/Library/Tpm2CommandLib/Tpm2Session.c    | 2 +-
> >   5 files changed, 11 insertions(+), 11 deletions(-)
> >
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-02-28  0:33 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-30 18:53 [Patch v2 0/3] Address C++ keyword collisions Michael D Kinney
2023-05-30 18:53 ` [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names Michael D Kinney
2023-05-31 18:17   ` Pedro Falcato
2023-05-31 18:41     ` Michael D Kinney
2023-06-01 15:01       ` Michael D Kinney
2023-06-06 17:56         ` Michael D Kinney
2023-06-06 21:01           ` [edk2-devel] " Pedro Falcato
2024-02-27 23:46           ` Michael D Kinney
2024-02-27 23:55             ` Pedro Falcato
2024-02-28  0:33               ` Michael D Kinney
2023-05-30 18:53 ` [Patch v2 2/3] SecurityPkg/Library/TpmCommandLib: Change xor to Xor Michael D Kinney
2023-05-31  2:38   ` [edk2-devel] [PATCH] ShellPkg\SmbiosView: SmBiosView does not print correct Slot ID information asperber
2023-05-30 18:53 ` [Patch v2 3/3] MdePkg/Include/IndustryStandard: Address C++ keyword collisions Michael D Kinney
2023-05-31  0:05 ` [edk2-devel] [Patch v2 0/3] " Yao, Jiewen
2023-05-31 17:46 ` Oliver Smith-Denny
2023-06-01  7:30   ` 回复: [edk2-devel] " gaoliming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox