[edk2-devel] [PATCH v1 0/5] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER

[Kun Qin]

Hi Marvin,

I would prefer not to rely on undefined behaviors from different 
compilers. Instead of using flexible arrays, is it better to remove the 
`Data` field, pack the structure and follow 
"VARIABLE_LOCK_ON_VAR_STATE_POLICY" pattern?

In that case, OFFSET_OF will be forced to change to sizeof, and 
read/write to `Data` will follow the range indicated by MessageLength. 
But yes, that will enforce developers to update their platform level 
implementations accordingly.

Regards,
Kun

On 06/23/2021 08:26, Laszlo Ersek wrote:
> On 06/23/21 08:54, Marvin Häuser wrote:
>> On 22.06.21 17:34, Laszlo Ersek wrote:
>>> On 06/18/21 11:37, Marvin Häuser wrote:
>>>> On 16.06.21 22:58, Kun Qin wrote:
>>>>> On 06/16/2021 00:02, Marvin Häuser wrote:
>>>>>> 2) Is it feasible yet with the current set of supported compilers to
>>>>>> support flexible arrays?
>>>>> My impression is that flexible arrays are already supported (as seen
>>>>> in UnitTestFrameworkPkg/PrivateInclude/UnitTestFrameworkTypes.h).
>>>>> Please correct me if I am wrong.
>>>>>
>>>>> Would you mind letting me know why this is applicable here? We are
>>>>> trying to seek ideas on how to catch developer mistakes caused by this
>>>>> change. So any input is appreciated.
>>>> Huh, interesting. Last time I tried I was told about incompatibilities
>>>> with MSVC, but I know some have been dropped since then (2005 and 2008
>>>> if I recall correctly?), so that'd be great to allow globally.
>>> I too am surprised to see
>>> "UnitTestFrameworkPkg/PrivateInclude/UnitTestFrameworkTypes.h". The
>>> flexible array member is a C99 feature, and I didn't even know that we
>>> disallowed it for the sake of particular VS toolchains -- I thought we
>>> had a more general reason than just "not supported by VS versions X
>>> and Y".
>>>
>>> The behavior of OFFSET_OF() would be interesting -- the OFFSET_OF()
>>> macro definition for non-gcc / non-clang:
>>>
>>> #define OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
>>>
>>> borders on undefined behavior as far as I can tell, so its behavior is
>>> totally up to the compiler. It works thus far okay on Visual Studio, but
>>> I couldn't say if it extended correctly to flexible array members.
>>
>> Yes, it's UB by the standard, but this is actually how MS implements
>> them (or used to anyway?). I don't see why it'd cause issues with
>> flexible arrays, as only the start of the array is relevant (which is
>> constant for all instances of the structure no matter the amount of
>> elements actually stored). Any specific concern? If so, they could be
>> addressed by appropriate STATIC_ASSERTs.
> 
> No specific concern; my point was that two aspects of the same "class"
> of undefined behavior didn't need to be consistent with each other.
> 
> Thanks
> Laszlo
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77017): https://edk2.groups.io/g/devel/message/77017
Mute This Topic: https://groups.io/mt/83435921/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-