From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@protonmail.com header.s=default header.b=EqnUiQD0; spf=pass (domain: protonmail.com, ip: 185.70.40.18, mailfrom: vit9696@protonmail.com) Received: from mail1.protonmail.ch (mail1.protonmail.ch [185.70.40.18]) by groups.io with SMTP; Wed, 14 Aug 2019 09:22:55 -0700 Date: Wed, 14 Aug 2019 16:22:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=default; t=1565799772; bh=3IgtyNbVktUzQUc1qpjffE9YTz0FWbRCv2x64EKt8j8=; h=Date:To:From:Cc:Reply-To:Subject:In-Reply-To:References: Feedback-ID:From; b=EqnUiQD0b+dvKt65H48ptiS9lN9ZXheDm66U1nfPt6YJvzz5oEJ6zKJytMfIfv7l4 V4kunV+EsiIBovbPLhqY6aNe/sAeclgLSPmJK9YPJuNw1hDXUKBpQ7ykQQPAq3zptN 8z7fj/kKJ3D6/XR6/XGmXnFfCj+7xP8z9BEoeups= To: michael.d.kinney@intel.com From: "Vitaly Cheptosv" Cc: "devel@edk2.groups.io" , Laszlo Ersek Reply-To: "vit9696@protonmail.com" Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro Message-ID: <65C2153C-7D59-490C-8DD2-A48FF0EEA8DE@protonmail.com> In-Reply-To: References: <20190813081644.53963-1-vit9696@protonmail.com> <20190813081644.53963-2-vit9696@protonmail.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E4D0EC7@SHSMSX104.ccr.corp.intel.com> Feedback-ID: p9QuX-L1wMgUm6nrSvNrf8juLupNs0VSnzXGVXuYDxlEahFdWtaedWDMB9zpwGDklGt7kzs1-RBc0cqz327Gcg==:Ext:ProtonMail MIME-Version: 1.0 X-Spam-Status: No, score=-0.7 required=7.0 tests=ALL_TRUSTED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM,FREEMAIL_REPLYTO_END_DIGIT autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mail.protonmail.ch X-Groupsio-MsgNum: 45622 Content-Type: multipart/signed; protocol="application/pgp-signature"; micalg=pgp-sha256; boundary="---------------------f70a0187d5e31761d576e14e82b3135b"; charset=UTF-8 -----------------------f70a0187d5e31761d576e14e82b3135b Cc: "devel@edk2.groups.io" , "Gao, Liming" , Laszlo Ersek Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Date: Wed, 14 Aug 2019 19:22:41 +0300 From: "vit9696@protonmail.com" In-Reply-To: Message-Id: <65C2153C-7D59-490C-8DD2-A48FF0EEA8DE@protonmail.com> Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) References: <20190813081644.53963-1-vit9696@protonmail.com> <20190813081644.53963-2-vit9696@protonmail.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E4D0EC7@SHSMSX104.ccr.corp.intel.com> <-oYbCGlXQcvXnnsL2bvwm0_mx-AEeT6WtLeV1BSw1EaJlVhkgz2DJB-HVThO2RqNbGASsqNeIq8Di8yMxklNEw==@protonmail.conversationid> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro To: "Kinney, Michael D" X-Mailer: Apple Mail (2.3445.104.11) Michael, Liming, Laszlo, Static assertions via _Static_assert are standard C11 functionality, thus = any at least C11 (ISO/IEC 9899 2011) conforming compiler is required to sup= port the second argument with the diagnostic description. The notation without the message currently is only present in C++, not in = C, thus the two-argument notation is the only allowed notation for _Static_= assert for at least C17 (ISO/IEC 9899 2018) and below. In the bottom of this message I included a quote from C17 for the relevant= section (6.7.10). GCC and CLANG (including Xcode) appear to be conforming to the standard fo= r this section, and MSVC compiler static_assert extension also supports the= diagnostic message argument. This is pretty much all we care about. As for examples, I see little reason to clarify STATIC_ASSERT behaviour ou= tside of the standard reference in its description and actual usage in the = source code, but can do that just fine if you think that it may help somebo= dy. I fully agree that VERIFY_SIZE_OF usage should be converted to STATIC_ASSE= RT, and in fact I also suggest VERIFY_SIZE_OF to be entirely removed from B= ase.h. This should be fairly costless, as apparently it is only used in Bas= e.h and MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c, which I can re= place in the same patch set. As for select ASSERT usage switching to STATIC_ASSERT, this would also be = great, as let us be honest, the use of ASSERT in EDK II codebase is very qu= estioning. In fact, this was one of the reasons we introduced our own stati= c assertions some time ago. However, fixing up all broken assertions is unl= ikely a best place to fit into this patchset, but I can surely add a few ex= amples, in case somebody points them out. This will be useful for reference= purposes and may help the maintainers to get a better idea when static ass= ertions are to be used. Looking forward to hearing your opinions. Best regards, Vitaly 6.7.10 Static assertions Syntax 1 static_assert-declaration: _Static_assert ( constant-expression , string-literal ) ; Constraints 2 The constant expression shall compare unequal to 0. Semantics 3 The constant expression shall be an integer constant expression. If the = value of the constant expression compares unequal to 0, the declaration has= no effect. Otherwise, the constraint is violated and the implementation sh= all produce a diagnostic message that includes the text of the string liter= al, except that characters not in the basic source character set are not re= quired to appear in the message. Forward references: diagnostics (7.2). 7.2 Diagnostics 3 The macro static_assert expands to _Static_assert. > 14 =D0=B0=D0=B2=D0=B3. 2019 =D0=B3., =D0=B2 18:47, Kinney, Michael D =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0=B0= ): >=20 >=20 > Liming, >=20 > I think a good candidate to demonstrate this > feature are the checks made in MdePkg/Include/Base.h. > The current implementation forces a divide by 0 > in the C pre-processor to break the build. > STATIC_ASSERT() would be a better way to do this. > I would also remove unused externs from the builds. >=20 > /** > Verifies the storage size of a given data type. >=20 > This macro generates a divide by zero error or a zero size array declar= ation in > the preprocessor if the size is incorrect. These are declared as "exte= rn" so > the space for these arrays will not be in the modules. >=20 > @param TYPE The date type to determine the size of. > @param Size The expected size for the TYPE. >=20 > **/ > #define VERIFY_SIZE_OF(TYPE, Size) extern UINT8 _VerifySizeof##TYPE[(siz= eof(TYPE) =3D=3D (Size)) / (sizeof(TYPE) =3D=3D (Size))] >=20 > // > // Verify that ProcessorBind.h produced UEFI Data Types that are complia= nt with > // Section 2.3.1 of the UEFI 2.3 Specification. > // > VERIFY_SIZE_OF (BOOLEAN, 1); > VERIFY_SIZE_OF (INT8, 1); > VERIFY_SIZE_OF (UINT8, 1); > VERIFY_SIZE_OF (INT16, 2); > VERIFY_SIZE_OF (UINT16, 2); > VERIFY_SIZE_OF (INT32, 4); > VERIFY_SIZE_OF (UINT32, 4); > VERIFY_SIZE_OF (INT64, 8); > VERIFY_SIZE_OF (UINT64, 8); > VERIFY_SIZE_OF (CHAR8, 1); > VERIFY_SIZE_OF (CHAR16, 2); >=20 > // > // The following three enum types are used to verify that the compiler > // configuration for enum types is compliant with Section 2.3.1 of the > // UEFI 2.3 Specification. These enum types and enum values are not > // intended to be used. A prefix of '__' is used avoid conflicts with > // other types. > // > typedef enum { > __VerifyUint8EnumValue =3D 0xff > } __VERIFY_UINT8_ENUM_SIZE; >=20 > typedef enum { > __VerifyUint16EnumValue =3D 0xffff > } __VERIFY_UINT16_ENUM_SIZE; >=20 > typedef enum { > __VerifyUint32EnumValue =3D 0xffffffff > } __VERIFY_UINT32_ENUM_SIZE; >=20 > VERIFY_SIZE_OF (__VERIFY_UINT8_ENUM_SIZE, 4); > VERIFY_SIZE_OF (__VERIFY_UINT16_ENUM_SIZE, 4); > VERIFY_SIZE_OF (__VERIFY_UINT32_ENUM_SIZE, 4); >=20 > A couple examples. Do all the compilers support the message parameter t= oo? >=20 > STATIC_ASSERT (sizeof (BOOLEAN) =3D=3D 1, "sizeof (BOOLEAN) does not mee= t UEFI Specification Data Type requirements") > STATIC_ASSERT (sizeof (UINT16) =3D=3D 2, "sizeof (UINT16) does not meet= UEFI Specification Data Type requirements") > STATIC_ASSERT (sizeof (INT32) =3D=3D 4, "sizeof (INT32) does not meet = UEFI Specification Data Type requirements") > STATIC_ASSERT (sizeof (CHAR16) =3D=3D 2, "sizeof (CHAR16) does not meet= UEFI Specification Data Type requirements") > STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) =3D=3D 4, "Size of enum= does not meet UEFI Specification Data Type requirements") > STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) =3D=3D 4, "Size of enu= m does not meet UEFI Specification Data Type requirements") >=20 > Thanks, >=20 > Mike >=20 >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] >> On Behalf Of Liming Gao >> Sent: Wednesday, August 14, 2019 6:50 AM >> To: devel@edk2.groups.io; vit9696@protonmail.com >> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add >> STATIC_ASSERT macro >>=20 >> Can you add the sample usage of new macro STATIC_ASSERT? >>=20 >> Or, give the link of static_assert or _Static_assert. >>=20 >> If so, the developer knows how to use them in source >> code. >>=20 >> Thanks >> Liming >>> -----Original Message----- >>> From: devel@edk2.groups.io >> [mailto:devel@edk2.groups.io] On Behalf Of >>> vit9696 via Groups.Io >>> Sent: Tuesday, August 13, 2019 4:17 PM >>> To: devel@edk2.groups.io >>> Subject: [edk2-devel] [PATCH v2 1/1] MdePkg: Add >> STATIC_ASSERT macro >>>=20 >>>=20 >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2048 >>>=20 >>> Provide a macro for compile time assertions. >>> Equivalent to C11 static_assert macro from assert.h. >>>=20 >>> Signed-off-by: Vitaly Cheptsov >> >>> --- >>> MdePkg/Include/Base.h | 11 +++++++++++ >>> 1 file changed, 11 insertions(+) >>>=20 >>> diff --git a/MdePkg/Include/Base.h >> b/MdePkg/Include/Base.h index >>> ce20b5f01dce..f85f7028a262 100644 >>> --- a/MdePkg/Include/Base.h >>> +++ b/MdePkg/Include/Base.h >>> @@ -843,6 +843,17 @@ typedef UINTN *BASE_LIST; >> #define >>> OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field)) >> #endif >>>=20 >>> +/// >>> +/// Portable definition for compile time assertions. >>> +/// Equivalent to C11 static_assert macro from >> assert.h. >>> +/// Takes condtion and error message as its >> arguments. >>> +/// >>> +#ifdef _MSC_EXTENSIONS >>> + #define STATIC_ASSERT static_assert #else >>> + #define STATIC_ASSERT _Static_assert #endif >>> + >>> /** >>> Macro that returns a pointer to the data structure >> that contains a specified field of >>> that data structure. This is a lightweight method >> to hide >>> information by placing a >>> -- >>> 2.20.1 (Apple Git-117) >>>=20 >>>=20 >>> -=3D-=3D-=3D-=3D-=3D-=3D >>> Groups.io Links: You receive all messages sent to this >> group. >>>=20 >>> View/Reply Online (#45503): >>> https://edk2.groups.io/g/devel/message/45503 >>> Mute This Topic: https://groups.io/mt/32850582/1759384 >>> Group Owner: devel+owner@edk2.groups.io >>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >>> [liming.gao@intel.com] -=3D-=3D-=3D-=3D-=3D-=3D >>=20 >>=20 >>=20 >=20 -----------------------f70a0187d5e31761d576e14e82b3135b Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: ProtonMail wsBmBAEBCAAQBQJdVDVSCRBPsoxt7Hy0xQAKCRBPsoxt7Hy0xUsiCAAcosZ8 Ea58UXnRmtbt62ERPEzd+ZFNyHzcJVW/k8emGNrxyI6yJTAssT0rbiCHp56J /t5a/sj5cu0/eGAeHezSN1gETfsn9gi6WuwtHRVpo4OEOLmkUar1yvx8vIzP SYi+mCXF3ZPdREyvwnkmqfZh+OIOq0QYN2p8A7OGDPemPMvAH4rpeXY1lke2 pm7S4QvTakUDwLsb38k0QyOF5RcZh+t8UA5/a7fFXoC8S3ryl28fGUOz2T2f lTEB2560ywAodjMZxSFXQH/xxVBgo8hms+bvxuUTKhL/3WXCf3ML85LwflIX Z7wL0346TO/FMxRoK41KyEfKFkVi65nirbt5 =ObMQ -----END PGP SIGNATURE----- -----------------------f70a0187d5e31761d576e14e82b3135b--