On 09/25/20 04:25, Bret Barkelew via groups.io wrote: > So for context, this is a new host-based test that should only run > within a platform OS, so intrinsics aren't the big deal that they > would be in FW code. > > But we do need to figure out how to simultaneously adhere to the > coding convention while enabling test authoring.
The EvaluatePolicyMatch() function -- very helpfully -- takes a const-qualified pointer to VARIABLE_POLICY_ENTRY. That allows me to see at once that the "MatchCheckPolicy" variable is not going to be modified, in the PoliciesShouldMatchByNameAndGuid() function [MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c]. So the solution is to give "MatchCheckPolicy" static storage duration rather than automatic. For good measure, make it CONST too: > diff --git > a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c > > b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c > index 40e946a73814..ab05989c36e7 100644 > --- > a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c > +++ > b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyUnitTest/VariablePolicyUnitTest.c > @@ -330,7 +330,7 @@ PoliciesShouldMatchByNameAndGuid ( > IN UNIT_TEST_CONTEXT Context > ) > { > - SIMPLE_VARIABLE_POLICY_ENTRY MatchCheckPolicy = { > + STATIC CONST SIMPLE_VARIABLE_POLICY_ENTRY MatchCheckPolicy = { > { > VARIABLE_POLICY_ENTRY_REVISION, > sizeof(VARIABLE_POLICY_ENTRY) + sizeof(TEST_VAR_1_NAME), In my opinion, this is an improvement *regardless* of the restrictions that edk2 places on compiler intrinsics. Namely, even in a hosted C environment, the actual data content needs to exist *somewhere* in the executable file, and correspondingly, in the executing process. And so when the struct with automatic storage duration is being initialized, there is either a large quick copy (cue the intrinsic), or -- naively -- a bunch of member-wise assignments behind the curtain (where the initializer values for the scalar fields could even be encoded as constants in the instruction stream). Either way, the data comes from *somewhere* -- so why not just *label* that original data with a name, and refer to the data by that name? And that's exactly what STATIC CONST does. (I realize the actual data content might end up with a different representation and/or in a different section of the on-disk executable, but that fact does not invalidate my point.) In case you do need the variable to be writeable and/or to have auto storage duration (e.g., because you need to fix up a few fields before use), the common pattern in edk2 is to have a template object (usually at file scope, not at function scope), such as: STATIC CONST SIMPLE_VARIABLE_POLICY_ENTRY mMatchCheckPolicy = { ... }; (note the "m" prefix on the name). Subsequently, wherever you need a writeable copy (local variable, or dynamically allocated object), use CopyMem() or AllocateCopyPool(), respectively. Then fix up any fields that require that. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65610): https://edk2.groups.io/g/devel/message/65610 Mute This Topic: https://groups.io/mt/77071159/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-