toggle quoted messageShow quoted text
Thanks for the information. I can update the patch and proposed spec change to use flexible array in v-next if there is no other concerns.
After switching to flexible array, OFFSET_OF (Data) should lead to the same result as sizeof. While sizeof would be a preferred way to move forward.
On 06/24/2021 08:25, Kinney, Michael D wrote:
Flexible array members are supported and should be used. The old style
of adding an array of size  at the end of a structure was used at a time
flexible array members were not supported by all compilers (late 1990's).
The workarounds used to handle the array of size  are very confusing when
reading the C code and the fact that sizeof() does not produce the expected
result make it even worse.
If we use flexible array members in this proposed change then there is
no need to use OFFSET_OF(). Correct?
From: Marvin Häuser <email@example.com>
Sent: Thursday, June 24, 2021 1:00 AM
To: Kun Qin <firstname.lastname@example.org>; Laszlo Ersek <email@example.com>; firstname.lastname@example.org
Cc: Wang, Jian J <email@example.com>; Wu, Hao A <firstname.lastname@example.org>; Dong, Eric <email@example.com>; Ni, Ray
<firstname.lastname@example.org>; Kinney, Michael D <email@example.com>; Liming Gao <firstname.lastname@example.org>; Liu, Zhiguang
<email@example.com>; Andrew Fish <firstname.lastname@example.org>; Leif Lindholm <email@example.com>; Bret Barkelew
Subject: Re: [edk2-devel] [PATCH v1 0/5] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER
Why would you rely on undefined behaviours? The OFFSET_OF macro is
well-defined for GCC and Clang as it's implemented by an intrinsic, and
while the expression for the MSVC compiler is undefined behaviour as per
the C standard, it is well-defined for MSVC due to their own
implementation being identical. From my standpoint, all supported
compilers will yield well-defined behaviour even this way. OFFSET_OF on
flexible arrays is not UB in any case to my knowledge.
However, the same way as your new suggestion, you can replace OFFSET_OF
with sizeof. While this *can* lead to wasted space with certain
structure layouts (e.g. when the flexible array overlays padding bytes),
this is not the case here, and otherwise just loses you a few bytes. I
think this comes down to preference.
The pattern you mentioned arguably is less nice syntax when used
(involves address calculation and casting), but the biggest problem here
is alignment constraints. For packed structures, you lose the ability of
automatic unaligned accesses (irrelevant here because the structure is
manually padded anyway). For non-packed structures, you still need to
ensure the alignment requirement of the trailing array data is met
manually. With flexible array members, the compiler takes care of both
On 24.06.21 02:24, Kun Qin wrote:
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
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
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:No specific concern; my point was that two aspects of the same "class"
On 06/18/21 11:37, Marvin Häuser wrote:Yes, it's UB by the standard, but this is actually how MS implements
On 16.06.21 22:58, Kun Qin wrote:I too am surprised to see
On 06/16/2021 00:02, Marvin Häuser wrote:Huh, interesting. Last time I tried I was told about
2) Is it feasible yet with the current set of supportedMy impression is that flexible arrays are already supported (as seen
support flexible arrays?
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
change. So any input is appreciated.
with MSVC, but I know some have been dropped since then (2005 and
if I recall correctly?), so that'd be great to allow globally.
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
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
I couldn't say if it extended correctly to flexible array members.
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.
of undefined behavior didn't need to be consistent with each other.