On 25/03/2020 18:40, Liran Alon wrote:
On 25/03/2020 18:31, Laszlo Ersek wrote:
On 03/25/20 02:11, Liran Alon wrote:
To avoid further style comments, what is the coding convention in EDK2
to align the "PVSCSI_CMD_DESC_SETUP_RINGS Cmd;" var properly?
The best I can recommend off-hand is:
union {
PVSCSI_CMD_DESC_SETUP_RINGS Cmd;
UINT32 Uint32;
} AlignAtUint32;
Perhaps someone else can recommend something less awkward.
Note: PVSCSI_CMD_DESC_SETUP_RINGS is a packed structure (and I do agree
that's good, if at least for documentation purposes). If it weren't
packed, then the following passage from the UEFI spec would apply:
2.3.1 Data Types
Table 5 lists the common data types that are used in the interface
definitions, and Table 6 lists their modifiers. Unless otherwise
specified all data types are naturally aligned. Structures are
aligned on boundaries equal to the largest internal datum of the
structure and internal data are implicitly padded to achieve
natural
alignment.
Because PVSCSI_CMD_DESC_SETUP_RINGS only contains members with types
listed in Table 5 (namely, UINT32 and UINT64), the above language would
normally guarantee the proper alignment. *But*, because the structure is
packed, I don't think we can rely on the spec's description (cf. "unless
otherwise specified").
So, in theory, there are two options:
- drop the packing (and rely on the natural alignment providing what you
need anyway),
- keep the packing, and use other methods to guarantee struct-level
alignment (such as the above union).
I prefer keeping the packing, if for nothing else then for documentation
purposes (it says "wire format" loud and clear). If you use the union
above, I'll be OK with it.
Ok. I will use this union approach.
Unfortunately, I have seen this comment only after submitting v2.
So I will wait for the rest of your v2 review comments and make sure
to do this change for v3 as-well.
Thanks.
Actually, I'm not sure I understand how this union approach helps with
anything.
Isn't the PVSCSI_CMD_DESC_SETUP_RINGS structure already aligned because
it have only UINT32 and UINT64 fields?
And if alignment is not guaranteed, how does putting it in a union
together with another UINT32 provides the required alignment it didn't
had before?
Because the union itself is not marked with packed(), in contrast to
PVSCSI_CMD_DESC_SETUP_RINGS?
-Liran
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56331): https://edk2.groups.io/g/devel/message/56331
Mute This Topic: https://groups.io/mt/72001280/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
