From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vk1-f176.google.com (mail-vk1-f176.google.com [209.85.221.176]) by mx.groups.io with SMTP id smtpd.web10.1705.1685557047497403622 for ; Wed, 31 May 2023 11:17:27 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="signature has expired" header.i=@gmail.com header.s=20221208 header.b=SzFu0MDw; spf=pass (domain: gmail.com, ip: 209.85.221.176, mailfrom: pedro.falcato@gmail.com) Received: by mail-vk1-f176.google.com with SMTP id 71dfb90a1353d-456de5aa485so1657941e0c.3 for ; Wed, 31 May 2023 11:17:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685557046; x=1688149046; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=CCDsMx38Pvz3P7wYPDlT4ayyjydD3CDk5Bc8n/mHGlo=; b=SzFu0MDwDSNYykD8skE8CvAZ7v8yaP2nz0N3beOZvnh5LZA8zRCZMKIJcZK69lVHob bApQy4WuxQGGenQhzVGrKHlIAXnHhjx4s8jl8+nda9/PMbGd9Iy1ircXfje/m7ikbh6P cK4nBurtcPTtryvY6g4HFiBdNtaMTD4aAe9WNlBcA+EBO8Id0Eg8mKZvS3f09XYjbNBK 1rzsnsU98eVFj+ZLR8gGsamI7OKi8HFYRWCFD4zkaOKm2MD+rb+PXv6UxmmgaBXDHlsQ BXIqlunrPVUwiyx2aUzTnWSBMsdZJ9ULo0O02GbCebrPCIv/XGCe1iQAYyiNHF9SpNhw RpYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685557046; x=1688149046; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CCDsMx38Pvz3P7wYPDlT4ayyjydD3CDk5Bc8n/mHGlo=; b=KM2WcoYulsnGcRF/fqjd/XS8OeTR3xbaup3v/ZTDSFrEw3luYG0lIshY+xwssS2hbG ldEX7GC3QKP0Xp/ONb08yzcgZtesgJbXrAB2NUvaRkvpHDgnLqJkRs24Jz7KpnjPv5Ed 0lxm24HkdcnSrFRtFEGWQP+l2xCSVSMEGfEOGfqxdXYhRIccJ95Atj7gKDIijvZFr2m0 Jvc6I8FVaIqQPaj97gwrBlakQq/4MLASeLDaLN8eJUCfprOkFH6jYYOxWUARKNet5CAt BWPWVCsEiiD7SeAxvY1HNFGoaTKOctQxgyssFdGYWEnT6WHzKnnmSO7UkgOmFmtyq5JO kTMA== X-Gm-Message-State: AC+VfDyAK+LeMFRlwlnHO8x04MDB/ytSSyI6KV1+8t6v74Cul2i1TKhJ py7Ui/AYzpx+WN8fn1CXr5e+aFc3ZB1eEFgEeKE= X-Google-Smtp-Source: ACHHUZ7onCU6HRTl1ViIRy+i3iD56Ho8c1pNZOid2GcA1+MNshniWOFkg3zLMQBZQ23WLS7a+NLcmOQn07Cx8Irwmds= X-Received: by 2002:a1f:4597:0:b0:456:fc81:1db3 with SMTP id s145-20020a1f4597000000b00456fc811db3mr10615vka.5.1685557046395; Wed, 31 May 2023 11:17:26 -0700 (PDT) MIME-Version: 1.0 References: <20230530185322.70-1-michael.d.kinney@intel.com> <20230530185322.70-2-michael.d.kinney@intel.com> In-Reply-To: <20230530185322.70-2-michael.d.kinney@intel.com> From: "Pedro Falcato" Date: Wed, 31 May 2023 19:17:15 +0100 Message-ID: Subject: Re: [Patch v2 1/3] MdePkg/Include/IndustryStandard: Add Operator and Xor field names To: Michael D Kinney Cc: devel@edk2.groups.io, Liming Gao , Zhiguang Liu , Oliver Smith-Denny , Aaron Pop Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, May 30, 2023 at 7:53=E2=80=AFPM Michael D Kinney 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 > Cc: Zhiguang Liu > Cc: Oliver Smith-Denny > Cc: Pedro Falcato > Cc: Aaron Pop > Signed-off-by: Michael D Kinney > --- > 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/Ind= ustryStandard/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=3Dc, 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/Ind= ustryStandard/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 > > +/// > +/// 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_? --=20 Pedro