Liming, Thank you for adding everyone to the CC list. Yes, I would like this to be merged into the next EDK II stable release.
Best regards, Vitaly On чт, авг. 15, 2019 at 04:59, Gao, Liming <liming....@intel.com> wrote: > Vitaly: > > As you know, edk2 201908 stable tag will start soft freeze tomorrow. Dose > this patch plan to catch this stable tag? > > If yes, please ask the feedback from Tianocore Stewards. I have cc this > patch to all Stewards. > > Thanks > > Liming[] > > []From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Yao, > Jiewen > Sent: Thursday, August 15, 2019 9:05 AM > To: devel@edk2.groups.io; vit9...@protonmail.com; Kinney, Michael D > <michael.d.kin...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro > > Good input. > > I think we should separate the work to convert all EDKII code to use > STATIC_ASSERT. > > We can do that work once we add STATIC_ASSERT. > > I recommend: > > 1) Step 1: Add STATIS_ASSERT - this patch and this Bugzilla > > 2) Step 2: Convert VERIFY_SIZE_OF to STATIS_ASSERT, and remove > VERIFY_SIZE_OF – the other patch and the other Bugzilla > > 3) Step 3: Scan the rest, if there is need. – Another patch and another > Bugzilla > > Thank you > > Yao Jiewen > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Vitaly > Cheptosv via Groups.Io > Sent: Thursday, August 15, 2019 12:23 AM > To: Kinney, Michael D <michael.d.kin...@intel.com> > Cc: devel@edk2.groups.io; Laszlo Ersek <ler...@redhat.com> > Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro > > 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 support > 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 for > 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 > outside 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 somebody. > > I fully agree that VERIFY_SIZE_OF usage should be converted to STATIC_ASSERT, > and in fact I also suggest VERIFY_SIZE_OF to be entirely removed from Base.h. > This should be fairly costless, as apparently it is only used in Base.h and > MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c, which I can replace 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 > questioning. In fact, this was one of the reasons we introduced our own > static assertions some time ago. However, fixing up all broken assertions is > unlikely a best place to fit into this patchset, but I can surely add a few > examples, 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 > assertions 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 shall > produce a diagnostic message that includes the text of the string literal, > except that characters not in the basic source character set are not required > to appear in the message. > > Forward references: diagnostics (7.2). > > 7.2 Diagnostics <assert. h> > > 3 The macro > static_assert > expands to _Static_assert. > >> 14авг. 2019 г., в 18:47, Kinney, Michael D <michael.d.kin...@intel.com> >> написал(а): >> >> >> Liming, >> >> 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. >> >> /** >> Verifies the storage size of a given data type. >> >> This macro generates a divide by zero error or a zero size array >> declaration in >> the preprocessor if the size is incorrect. These are declared as "extern" >> so >> the space for these arrays will not be in the modules. >> >> @param TYPE The date type to determine the size of. >> @param Size The expected size for the TYPE. >> >> **/ >> #define VERIFY_SIZE_OF(TYPE, Size) extern UINT8 >> _VerifySizeof##TYPE[(sizeof(TYPE) == (Size)) / (sizeof(TYPE) == (Size))] > >> >> // >> // Verify that ProcessorBind.h produced UEFI Data Types that are compliant >> 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); >> >> // >> // 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 = 0xff >> } __VERIFY_UINT8_ENUM_SIZE; >> >> typedef enum { >> __VerifyUint16EnumValue = 0xffff >> } __VERIFY_UINT16_ENUM_SIZE; >> >> typedef enum { >> __VerifyUint32EnumValue = 0xffffffff >> } __VERIFY_UINT32_ENUM_SIZE; >> >> VERIFY_SIZE_OF (__VERIFY_UINT8_ENUM_SIZE, 4); >> VERIFY_SIZE_OF (__VERIFY_UINT16_ENUM_SIZE, 4); >> VERIFY_SIZE_OF (__VERIFY_UINT32_ENUM_SIZE, 4); >> >> A couple examples. Do all the compilers support the message parameter too? >> >> STATIC_ASSERT (sizeof (BOOLEAN) == 1, "sizeof (BOOLEAN) does not meet UEFI >> Specification Data Type requirements") >> STATIC_ASSERT (sizeof (UINT16) == 2, "sizeof (UINT16) does not meet UEFI >> Specification Data Type requirements") >> STATIC_ASSERT (sizeof (INT32) == 4, "sizeof (INT32) does not meet UEFI >> Specification Data Type requirements") >> STATIC_ASSERT (sizeof (CHAR16) == 2, "sizeof (CHAR16) does not meet UEFI >> Specification Data Type requirements") >> STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) == 4, "Size of enum does >> not meet UEFI Specification Data Type requirements") > >> STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does >> not meet UEFI Specification Data Type requirements") > >> >> Thanks, >> >> Mike >> >>> -----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; vit9...@protonmail.com >>> Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add >>> STATIC_ASSERT macro >>> >>> Can you add the sample usage of new macro STATIC_ASSERT? >>> >>> Or, give the link of static_assert or _Static_assert. >>> >>> If so, the developer knows how to use them in source >>> code. >>> >>> 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 >>>> >>>> >>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2048 >>>> >>>> Provide a macro for compile time assertions. >>>> Equivalent to C11 static_assert macro from assert.h. >>>> >>>> Signed-off-by: Vitaly Cheptsov >>> <vit9...@protonmail.com> >>>> --- >>>> MdePkg/Include/Base.h | 11 +++++++++++ >>>> 1 file changed, 11 insertions(+) >>>> >>>> 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 >>>> >>>> +/// >>>> +/// 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) >>>> >>>> >>>> -=-=-=-=-=-= >>>> Groups.io Links: You receive all messages sent to this >>> group. >>>> >>>> View/Reply Online (#45503): >>>> https://edk2.groups.io/g/devel/message/45503 >>>> Mute This Topic: https://groups.io/mt/32850582/1759384 >>>> Group Owner: devel+ow...@edk2.groups.io >>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub >>>> [liming....@intel.com] -=-=-=-=-=-= >>> >>> >>> >> > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#45651): https://edk2.groups.io/g/devel/message/45651 Mute This Topic: https://groups.io/mt/32850582/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
