VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Bret Barkelew]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

I’ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>

I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>

However, there’s one that I just can’t get past and I would like to take it up with the community. I don’t think that UnitTests should have to deal with the “can’t initialize variables in declaration” check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:

  *   They were annoying to write ===> fewer tests.
  *   They moved even more of the test case data away from the test ===> harder to read tests.

I would like to move for an exception for unit tests (or at least host-based unit tests), but I don’t know how to accomplish that from a technical standpoint.

Thoughts?

- Bret


[-- Attachment #2: Type: text/html, Size: 5456 bytes --]

Re: VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 2319 bytes --]


Bret,

Initializing variable in declaration for structures and arrays introduces use of intrinsics.  Since it is possible for unit test sources to be used for both host and target tests, I recommend we continue to follow the EDK II coding style for unit tests to support maximum compatibility and code reuse.

Using a module global variable with initializers instead of initializing a local declaration is the same amount of work, so I do not believe that will result in fewer tests.

I agree it is useful to have the test data next to the test code.  This can be accomplished by breaking up into more files so the test data is immediately above the test function the test data is used.  Does ECC raise an error if a module global is placed between 2 functions?  A 2nd approach to put the module global immediately above the test function the test data is used.

Best regards,

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
Sent: Tuesday, October 6, 2020 5:28 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

I’ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>

I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>

However, there’s one that I just can’t get past and I would like to take it up with the community. I don’t think that UnitTests should have to deal with the “can’t initialize variables in declaration” check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:

  *   They were annoying to write ===> fewer tests.
  *   They moved even more of the test case data away from the test ===> harder to read tests.

I would like to move for an exception for unit tests (or at least host-based unit tests), but I don’t know how to accomplish that from a technical standpoint.

Thoughts?

- Bret



[-- Attachment #2: Type: text/html, Size: 47953 bytes --]

Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Laszlo Ersek]

On 10/07/20 03:46, Michael D Kinney wrote:
>
> Bret,
>
> Initializing variable in declaration for structures and arrays
> introduces use of intrinsics.  Since it is possible for unit test
> sources to be used for both host and target tests, I recommend we
> continue to follow the EDK II coding style for unit tests to support
> maximum compatibility and code reuse.
>
> Using a module global variable with initializers instead of
> initializing a local declaration is the same amount of work, so I do
> not believe that will result in fewer tests.
>
> I agree it is useful to have the test data next to the test code. This
> can be accomplished by breaking up into more files so the test data is
> immediately above the test function the test data is used.  Does ECC
> raise an error if a module global is placed between 2 functions?  A
> 2nd approach to put the module global immediately above the test
> function the test data is used.

Consider the following example structure type, for the sake of
discussion:

  typedef struct {
    UINT32 Value;
  } TEST_DATA;


* Case#1: block scope, automatic storage duration

  EFI_STATUS
  FoobarTest (
    VOID
    )
  {
    TEST_DATA TestData = { 42 };
    // ...
  }

Problem: uses intrinsics.


* Case#2: file scope, static storage duration.

  STATIC CONST TEST_DATA mTestData = { 42 };

  EFI_STATUS
  FoobarTest (
    VOID
    )
  {
    // ...
  }

Problem: either "mTestData" is textually far from FoobarTest(), or -- if
we keep them close to each other -- we mix variable definitions with
function definitions, at file scope.


* Case #3: block scope, static storage duration.

  EFI_STATUS
  FoobarTest (
    VOID
    )
  {
    STATIC CONST TEST_DATA TestData = { 42 };
    // ...
  }

Problem: there should be none. Does not involve intrinsics, and the
object definition is part of the function's scope.


If ECC does not recognize case#3 as valid, then that is an *ECC bug*.

ECC has no reason to prevent case#3, as case#3 does not involve
intrinsics, and is a generally valid and useful C language construct (it
combines the life cycle of case#2 with the visibility of case#1).

Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
should fix it first. Given that ECC includes a full-blown C language
parser, the fix should not be too difficult -- check if the declaration
has the "static" storage-class specifier.

... In fact, I think that purely CONST-qualifying TestData might suffice
for shutting up ECC. See the following in
"BaseTools/Source/Python/Ecc/c.py", method
"CheckFuncLayoutLocalVariable":

>         for Result in ResultSet:
>             if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>                 PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])

So case#3 should work through that avenue already, because case#3 has
CONST *too*.

Now, in case#3, if "TestData" needs to undergo modifications, and so
CONST is not immediately desirable, that's solvable:

  EFI_STATUS
  FoobarTest (
    VOID
    )
  {
    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
    TEST_DATA TestData;

    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
    // ...
  }

Thanks
Laszlo

>
> Best regards,
>
> Mike
>
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
> Sent: Tuesday, October 6, 2020 5:28 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>
> I\x19ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
> Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>
>
> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>
>
> However, there\x19s one that I just can\x19t get past and I would like to take it up with the community. I don\x19t think that UnitTests should have to deal with the \x1ccan\x19t initialize variables in declaration\x1d check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>
>   *   They were annoying to write ===> fewer tests.
>   *   They moved even more of the test case data away from the test ===> harder to read tests.
>
> I would like to move for an exception for unit tests (or at least host-based unit tests), but I don\x19t know how to accomplish that from a technical standpoint.
>
> Thoughts?
>
> - Bret
>
>
>
>
> 
>
>

Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Andrew Fish]

For case 1 I thought the size had to be > 8 bytes, not just a struct? Maybe that is compiler specific?
> On Oct 7, 2020, at 6:43 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 10/07/20 03:46, Michael D Kinney wrote:
>> 
>> Bret,
>> 
>> Initializing variable in declaration for structures and arrays
>> introduces use of intrinsics.  Since it is possible for unit test
>> sources to be used for both host and target tests, I recommend we
>> continue to follow the EDK II coding style for unit tests to support
>> maximum compatibility and code reuse.
>> 
>> Using a module global variable with initializers instead of
>> initializing a local declaration is the same amount of work, so I do
>> not believe that will result in fewer tests.
>> 
>> I agree it is useful to have the test data next to the test code. This
>> can be accomplished by breaking up into more files so the test data is
>> immediately above the test function the test data is used.  Does ECC
>> raise an error if a module global is placed between 2 functions?  A
>> 2nd approach to put the module global immediately above the test
>> function the test data is used.
> 
> Consider the following example structure type, for the sake of
> discussion:
> 
>  typedef struct {
>    UINT32 Value;
>  } TEST_DATA;
> 
> 
> * Case#1: block scope, automatic storage duration
> 
>  EFI_STATUS
>  FoobarTest (
>    VOID
>    )
>  {
>    TEST_DATA TestData = { 42 };
>    // ...
>  }
> 
> Problem: uses intrinsics.
> 
> 
> * Case#2: file scope, static storage duration.
> 
>  STATIC CONST TEST_DATA mTestData = { 42 };
> 
>  EFI_STATUS
>  FoobarTest (
>    VOID
>    )
>  {
>    // ...
>  }
> 
> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
> we keep them close to each other -- we mix variable definitions with
> function definitions, at file scope.
> 
> 
> * Case #3: block scope, static storage duration.
> 
>  EFI_STATUS
>  FoobarTest (
>    VOID
>    )
>  {
>    STATIC CONST TEST_DATA TestData = { 42 };
>    // ...
>  }
> 
> Problem: there should be none. Does not involve intrinsics, and the
> object definition is part of the function's scope.
> 
> 
> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
> 
> ECC has no reason to prevent case#3, as case#3 does not involve
> intrinsics, and is a generally valid and useful C language construct (it
> combines the life cycle of case#2 with the visibility of case#1).
> 
> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
> should fix it first. Given that ECC includes a full-blown C language
> parser, the fix should not be too difficult -- check if the declaration
> has the "static" storage-class specifier.
> 
> ... In fact, I think that purely CONST-qualifying TestData might suffice
> for shutting up ECC. See the following in
> "BaseTools/Source/Python/Ecc/c.py", method
> "CheckFuncLayoutLocalVariable":
> 
>>        for Result in ResultSet:
>>            if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>>                PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])
> 
> So case#3 should work through that avenue already, because case#3 has
> CONST *too*.
> 
> Now, in case#3, if "TestData" needs to undergo modifications, and so
> CONST is not immediately desirable, that's solvable:
> 
>  EFI_STATUS
>  FoobarTest (
>    VOID
>    )
>  {
>    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>    TEST_DATA TestData;
> 
>    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>    // ...
>  }
> 
> Thanks
> Laszlo
> 
>> 
>> Best regards,
>> 
>> Mike
>> 
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
>> Sent: Tuesday, October 6, 2020 5:28 PM
>> To: devel@edk2.groups.io
>> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>> 
>> I\x19ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
>> Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>
>> 
>> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
>> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>
>> 
>> However, there\x19s one that I just can\x19t get past and I would like to take it up with the community. I don\x19t think that UnitTests should have to deal with the \x1ccan\x19t initialize variables in declaration\x1d check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>> 
>>  *   They were annoying to write ===> fewer tests.
>>  *   They moved even more of the test case data away from the test ===> harder to read tests.
>> 
>> I would like to move for an exception for unit tests (or at least host-based unit tests), but I don\x19t know how to accomplish that from a technical standpoint.
>> 
>> Thoughts?
>> 
>> - Bret
>> 
>> 
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Laszlo Ersek]

On 10/07/20 16:27, Andrew Fish wrote:
> For case 1 I thought the size had to be > 8 bytes, not just a struct? Maybe that is compiler specific?

Honestly, I've got no clue. I just remember we must avoid initializers
for objects that do not have static storage duration.

Laszlo

> 
> Sent from my iPhone
> 
>> On Oct 7, 2020, at 6:43 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 10/07/20 03:46, Michael D Kinney wrote:
>>>
>>> Bret,
>>>
>>> Initializing variable in declaration for structures and arrays
>>> introduces use of intrinsics.  Since it is possible for unit test
>>> sources to be used for both host and target tests, I recommend we
>>> continue to follow the EDK II coding style for unit tests to support
>>> maximum compatibility and code reuse.
>>>
>>> Using a module global variable with initializers instead of
>>> initializing a local declaration is the same amount of work, so I do
>>> not believe that will result in fewer tests.
>>>
>>> I agree it is useful to have the test data next to the test code. This
>>> can be accomplished by breaking up into more files so the test data is
>>> immediately above the test function the test data is used.  Does ECC
>>> raise an error if a module global is placed between 2 functions?  A
>>> 2nd approach to put the module global immediately above the test
>>> function the test data is used.
>>
>> Consider the following example structure type, for the sake of
>> discussion:
>>
>>  typedef struct {
>>    UINT32 Value;
>>  } TEST_DATA;
>>
>>
>> * Case#1: block scope, automatic storage duration
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: uses intrinsics.
>>
>>
>> * Case#2: file scope, static storage duration.
>>
>>  STATIC CONST TEST_DATA mTestData = { 42 };
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    // ...
>>  }
>>
>> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
>> we keep them close to each other -- we mix variable definitions with
>> function definitions, at file scope.
>>
>>
>> * Case #3: block scope, static storage duration.
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: there should be none. Does not involve intrinsics, and the
>> object definition is part of the function's scope.
>>
>>
>> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
>>
>> ECC has no reason to prevent case#3, as case#3 does not involve
>> intrinsics, and is a generally valid and useful C language construct (it
>> combines the life cycle of case#2 with the visibility of case#1).
>>
>> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
>> should fix it first. Given that ECC includes a full-blown C language
>> parser, the fix should not be too difficult -- check if the declaration
>> has the "static" storage-class specifier.
>>
>> ... In fact, I think that purely CONST-qualifying TestData might suffice
>> for shutting up ECC. See the following in
>> "BaseTools/Source/Python/Ecc/c.py", method
>> "CheckFuncLayoutLocalVariable":
>>
>>>        for Result in ResultSet:
>>>            if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>>>                PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])
>>
>> So case#3 should work through that avenue already, because case#3 has
>> CONST *too*.
>>
>> Now, in case#3, if "TestData" needs to undergo modifications, and so
>> CONST is not immediately desirable, that's solvable:
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>>    TEST_DATA TestData;
>>
>>    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>>    // ...
>>  }
>>
>> Thanks
>> Laszlo
>>
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
>>> Sent: Tuesday, October 6, 2020 5:28 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>>>
>>> I\x19ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
>>> Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>
>>>
>>> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
>>> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>
>>>
>>> However, there\x19s one that I just can\x19t get past and I would like to take it up with the community. I don\x19t think that UnitTests should have to deal with the \x1ccan\x19t initialize variables in declaration\x1d check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>>>
>>>  *   They were annoying to write ===> fewer tests.
>>>  *   They moved even more of the test case data away from the test ===> harder to read tests.
>>>
>>> I would like to move for an exception for unit tests (or at least host-based unit tests), but I don\x19t know how to accomplish that from a technical standpoint.
>>>
>>> Thoughts?
>>>
>>> - Bret
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> 
>>
>>
> 

Re: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Bret Barkelew]

[-- Attachment #1: Type: text/plain, Size: 7195 bytes --]

I can test it again, but I hit a compiler complaint around the STATIC CONST solution.
Also, it doesn’t apply in all cases because I have at lease half a dozen cases that say “test X, prove negative, twiddle value, test Y, prove positive”.

I’ll try a few more things, but I may reissue the patch series and withhold the tests until they can be rewritten to match. I don’t want them to hold up VarPol any longer.

- Bret

From: Laszlo Ersek<mailto:lersek@redhat.com>
Sent: Wednesday, October 7, 2020 8:51 AM
To: Andrew Fish<mailto:afish@apple.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Kinney, Michael D<mailto:michael.d.kinney@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
Subject: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

On 10/07/20 16:27, Andrew Fish wrote:
> For case 1 I thought the size had to be > 8 bytes, not just a struct? Maybe that is compiler specific?

Honestly, I've got no clue. I just remember we must avoid initializers
for objects that do not have static storage duration.

Laszlo

>
> Sent from my iPhone
>
>> On Oct 7, 2020, at 6:43 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 10/07/20 03:46, Michael D Kinney wrote:
>>>
>>> Bret,
>>>
>>> Initializing variable in declaration for structures and arrays
>>> introduces use of intrinsics.  Since it is possible for unit test
>>> sources to be used for both host and target tests, I recommend we
>>> continue to follow the EDK II coding style for unit tests to support
>>> maximum compatibility and code reuse.
>>>
>>> Using a module global variable with initializers instead of
>>> initializing a local declaration is the same amount of work, so I do
>>> not believe that will result in fewer tests.
>>>
>>> I agree it is useful to have the test data next to the test code. This
>>> can be accomplished by breaking up into more files so the test data is
>>> immediately above the test function the test data is used.  Does ECC
>>> raise an error if a module global is placed between 2 functions?  A
>>> 2nd approach to put the module global immediately above the test
>>> function the test data is used.
>>
>> Consider the following example structure type, for the sake of
>> discussion:
>>
>>  typedef struct {
>>    UINT32 Value;
>>  } TEST_DATA;
>>
>>
>> * Case#1: block scope, automatic storage duration
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: uses intrinsics.
>>
>>
>> * Case#2: file scope, static storage duration.
>>
>>  STATIC CONST TEST_DATA mTestData = { 42 };
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    // ...
>>  }
>>
>> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
>> we keep them close to each other -- we mix variable definitions with
>> function definitions, at file scope.
>>
>>
>> * Case #3: block scope, static storage duration.
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: there should be none. Does not involve intrinsics, and the
>> object definition is part of the function's scope.
>>
>>
>> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
>>
>> ECC has no reason to prevent case#3, as case#3 does not involve
>> intrinsics, and is a generally valid and useful C language construct (it
>> combines the life cycle of case#2 with the visibility of case#1).
>>
>> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
>> should fix it first. Given that ECC includes a full-blown C language
>> parser, the fix should not be too difficult -- check if the declaration
>> has the "static" storage-class specifier.
>>
>> ... In fact, I think that purely CONST-qualifying TestData might suffice
>> for shutting up ECC. See the following in
>> "BaseTools/Source/Python/Ecc/c.py", method
>> "CheckFuncLayoutLocalVariable":
>>
>>>        for Result in ResultSet:
>>>            if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>>>                PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])
>>
>> So case#3 should work through that avenue already, because case#3 has
>> CONST *too*.
>>
>> Now, in case#3, if "TestData" needs to undergo modifications, and so
>> CONST is not immediately desirable, that's solvable:
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>>    TEST_DATA TestData;
>>
>>    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>>    // ...
>>  }
>>
>> Thanks
>> Laszlo
>>
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
>>> Sent: Tuesday, October 6, 2020 5:28 PM
>>> To: devel@edk2.groups.io
>>> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>>>
>>> Ive worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
>>> Commits · corthon/edk2 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommits%2Fvar_policy_dev_submission_v8&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=XSNAxTGdbKGlrPLV%2BqWVNEAQjyyeSoKQ8zcfG1%2B4W1s%3D&amp;reserved=0>
>>>
>>> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
>>> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommit%2F4ce52108b3e1bcb2ba78995be94c3949fe647eda&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=46MZeja1WTnq44vSZOwJQ%2BAs61TbdFQtFfyj7wgm5mY%3D&amp;reserved=0>
>>>
>>> However, theres one that I just cant get past and I would like to take it up with the community. I dont think that UnitTests should have to deal with the \x1ccant initialize variables in declaration check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>>>
>>>  *   They were annoying to write ===> fewer tests.
>>>  *   They moved even more of the test case data away from the test ===> harder to read tests.
>>>
>>> I would like to move for an exception for unit tests (or at least host-based unit tests), but I dont know how to accomplish that from a technical standpoint.
>>>
>>> Thoughts?
>>>
>>> - Bret
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> 
>>
>>
>


[-- Attachment #2: Type: text/html, Size: 11615 bytes --]

Re: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Michael D Kinney]

[-- Attachment #1: Type: text/plain, Size: 7777 bytes --]

Bret,

Can you provide the compiler log?

You can still use STATIC CONST and copy it to a 2nd local that allows mods.

Mike

From: Bret Barkelew <Bret.Barkelew@microsoft.com>
Sent: Wednesday, October 7, 2020 9:45 AM
To: Laszlo Ersek <lersek@redhat.com>; Andrew Fish <afish@apple.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

I can test it again, but I hit a compiler complaint around the STATIC CONST solution.
Also, it doesn’t apply in all cases because I have at lease half a dozen cases that say “test X, prove negative, twiddle value, test Y, prove positive”.

I’ll try a few more things, but I may reissue the patch series and withhold the tests until they can be rewritten to match. I don’t want them to hold up VarPol any longer.

- Bret

From: Laszlo Ersek<mailto:lersek@redhat.com>
Sent: Wednesday, October 7, 2020 8:51 AM
To: Andrew Fish<mailto:afish@apple.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Kinney, Michael D<mailto:michael.d.kinney@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
Subject: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

On 10/07/20 16:27, Andrew Fish wrote:
> For case 1 I thought the size had to be > 8 bytes, not just a struct? Maybe that is compiler specific?

Honestly, I've got no clue. I just remember we must avoid initializers
for objects that do not have static storage duration.

Laszlo

>
> Sent from my iPhone
>
>> On Oct 7, 2020, at 6:43 AM, Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> wrote:
>>
>> On 10/07/20 03:46, Michael D Kinney wrote:
>>>
>>> Bret,
>>>
>>> Initializing variable in declaration for structures and arrays
>>> introduces use of intrinsics.  Since it is possible for unit test
>>> sources to be used for both host and target tests, I recommend we
>>> continue to follow the EDK II coding style for unit tests to support
>>> maximum compatibility and code reuse.
>>>
>>> Using a module global variable with initializers instead of
>>> initializing a local declaration is the same amount of work, so I do
>>> not believe that will result in fewer tests.
>>>
>>> I agree it is useful to have the test data next to the test code. This
>>> can be accomplished by breaking up into more files so the test data is
>>> immediately above the test function the test data is used.  Does ECC
>>> raise an error if a module global is placed between 2 functions?  A
>>> 2nd approach to put the module global immediately above the test
>>> function the test data is used.
>>
>> Consider the following example structure type, for the sake of
>> discussion:
>>
>>  typedef struct {
>>    UINT32 Value;
>>  } TEST_DATA;
>>
>>
>> * Case#1: block scope, automatic storage duration
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: uses intrinsics.
>>
>>
>> * Case#2: file scope, static storage duration.
>>
>>  STATIC CONST TEST_DATA mTestData = { 42 };
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    // ...
>>  }
>>
>> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
>> we keep them close to each other -- we mix variable definitions with
>> function definitions, at file scope.
>>
>>
>> * Case #3: block scope, static storage duration.
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestData = { 42 };
>>    // ...
>>  }
>>
>> Problem: there should be none. Does not involve intrinsics, and the
>> object definition is part of the function's scope.
>>
>>
>> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
>>
>> ECC has no reason to prevent case#3, as case#3 does not involve
>> intrinsics, and is a generally valid and useful C language construct (it
>> combines the life cycle of case#2 with the visibility of case#1).
>>
>> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
>> should fix it first. Given that ECC includes a full-blown C language
>> parser, the fix should not be too difficult -- check if the declaration
>> has the "static" storage-class specifier.
>>
>> ... In fact, I think that purely CONST-qualifying TestData might suffice
>> for shutting up ECC. See the following in
>> "BaseTools/Source/Python/Ecc/c.py", method
>> "CheckFuncLayoutLocalVariable":
>>
>>>        for Result in ResultSet:
>>>            if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>>>                PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])
>>
>> So case#3 should work through that avenue already, because case#3 has
>> CONST *too*.
>>
>> Now, in case#3, if "TestData" needs to undergo modifications, and so
>> CONST is not immediately desirable, that's solvable:
>>
>>  EFI_STATUS
>>  FoobarTest (
>>    VOID
>>    )
>>  {
>>    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>>    TEST_DATA TestData;
>>
>>    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>>    // ...
>>  }
>>
>> Thanks
>> Laszlo
>>
>>>
>>> Best regards,
>>>
>>> Mike
>>>
>>> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Bret Barkelew via groups.io
>>> Sent: Tuesday, October 6, 2020 5:28 PM
>>> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
>>> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>>>
>>> Ive worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
>>> Commits · corthon/edk2 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommits%2Fvar_policy_dev_submission_v8&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=XSNAxTGdbKGlrPLV%2BqWVNEAQjyyeSoKQ8zcfG1%2B4W1s%3D&amp;reserved=0>
>>>
>>> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
>>> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommit%2F4ce52108b3e1bcb2ba78995be94c3949fe647eda&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=46MZeja1WTnq44vSZOwJQ%2BAs61TbdFQtFfyj7wgm5mY%3D&amp;reserved=0>
>>>
>>> However, theres one that I just cant get past and I would like to take it up with the community. I dont think that UnitTests should have to deal with the \x1ccant initialize variables in declaration check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>>>
>>>  *   They were annoying to write ===> fewer tests.
>>>  *   They moved even more of the test case data away from the test ===> harder to read tests.
>>>
>>> I would like to move for an exception for unit tests (or at least host-based unit tests), but I dont know how to accomplish that from a technical standpoint.
>>>
>>> Thoughts?
>>>
>>> - Bret
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>> 
>>
>>
>


[-- Attachment #2: Type: text/html, Size: 50792 bytes --]

Re: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Laszlo Ersek]

On 10/07/20 18:44, Bret Barkelew wrote:
> I can test it again, but I hit a compiler complaint around the STATIC CONST solution.

I'm curious.

> Also, it doesn’t apply in all cases because I have at lease half a dozen cases that say “test X, prove negative, twiddle value, test Y, prove positive”.

That works with the last example I gave; "STATIC CONST TestDataTemplate"
+ normal local TestData. CopyMem from template to local variable, check
and tweak local var as needed. Please check the end of my previous email
again.

  EFI_STATUS
  FoobarTest (
    VOID
    )
  {
    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
    TEST_DATA TestData;

    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
    //
    // run a test on TestData, verify that it fails
    //

    TestData.Value = 43;
    //
    // run the same test on TestData, verify that it succeeds
    //
}

thanks
Laszlo

> 
> I’ll try a few more things, but I may reissue the patch series and withhold the tests until they can be rewritten to match. I don’t want them to hold up VarPol any longer.
> 
> - Bret
> 
> From: Laszlo Ersek<mailto:lersek@redhat.com>
> Sent: Wednesday, October 7, 2020 8:51 AM
> To: Andrew Fish<mailto:afish@apple.com>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Kinney, Michael D<mailto:michael.d.kinney@intel.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>
> Subject: [EXTERNAL] Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
> 
> On 10/07/20 16:27, Andrew Fish wrote:
>> For case 1 I thought the size had to be > 8 bytes, not just a struct? Maybe that is compiler specific?
> 
> Honestly, I've got no clue. I just remember we must avoid initializers
> for objects that do not have static storage duration.
> 
> Laszlo
> 
>>
>> Sent from my iPhone
>>
>>> On Oct 7, 2020, at 6:43 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>>
>>> On 10/07/20 03:46, Michael D Kinney wrote:
>>>>
>>>> Bret,
>>>>
>>>> Initializing variable in declaration for structures and arrays
>>>> introduces use of intrinsics.  Since it is possible for unit test
>>>> sources to be used for both host and target tests, I recommend we
>>>> continue to follow the EDK II coding style for unit tests to support
>>>> maximum compatibility and code reuse.
>>>>
>>>> Using a module global variable with initializers instead of
>>>> initializing a local declaration is the same amount of work, so I do
>>>> not believe that will result in fewer tests.
>>>>
>>>> I agree it is useful to have the test data next to the test code. This
>>>> can be accomplished by breaking up into more files so the test data is
>>>> immediately above the test function the test data is used.  Does ECC
>>>> raise an error if a module global is placed between 2 functions?  A
>>>> 2nd approach to put the module global immediately above the test
>>>> function the test data is used.
>>>
>>> Consider the following example structure type, for the sake of
>>> discussion:
>>>
>>>  typedef struct {
>>>    UINT32 Value;
>>>  } TEST_DATA;
>>>
>>>
>>> * Case#1: block scope, automatic storage duration
>>>
>>>  EFI_STATUS
>>>  FoobarTest (
>>>    VOID
>>>    )
>>>  {
>>>    TEST_DATA TestData = { 42 };
>>>    // ...
>>>  }
>>>
>>> Problem: uses intrinsics.
>>>
>>>
>>> * Case#2: file scope, static storage duration.
>>>
>>>  STATIC CONST TEST_DATA mTestData = { 42 };
>>>
>>>  EFI_STATUS
>>>  FoobarTest (
>>>    VOID
>>>    )
>>>  {
>>>    // ...
>>>  }
>>>
>>> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
>>> we keep them close to each other -- we mix variable definitions with
>>> function definitions, at file scope.
>>>
>>>
>>> * Case #3: block scope, static storage duration.
>>>
>>>  EFI_STATUS
>>>  FoobarTest (
>>>    VOID
>>>    )
>>>  {
>>>    STATIC CONST TEST_DATA TestData = { 42 };
>>>    // ...
>>>  }
>>>
>>> Problem: there should be none. Does not involve intrinsics, and the
>>> object definition is part of the function's scope.
>>>
>>>
>>> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
>>>
>>> ECC has no reason to prevent case#3, as case#3 does not involve
>>> intrinsics, and is a generally valid and useful C language construct (it
>>> combines the life cycle of case#2 with the visibility of case#1).
>>>
>>> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
>>> should fix it first. Given that ECC includes a full-blown C language
>>> parser, the fix should not be too difficult -- check if the declaration
>>> has the "static" storage-class specifier.
>>>
>>> ... In fact, I think that purely CONST-qualifying TestData might suffice
>>> for shutting up ECC. See the following in
>>> "BaseTools/Source/Python/Ecc/c.py", method
>>> "CheckFuncLayoutLocalVariable":
>>>
>>>>        for Result in ResultSet:
>>>>            if len(Result[1]) > 0 and 'CONST' not in Result[3]:
>>>>                PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2])
>>>
>>> So case#3 should work through that avenue already, because case#3 has
>>> CONST *too*.
>>>
>>> Now, in case#3, if "TestData" needs to undergo modifications, and so
>>> CONST is not immediately desirable, that's solvable:
>>>
>>>  EFI_STATUS
>>>  FoobarTest (
>>>    VOID
>>>    )
>>>  {
>>>    STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>>>    TEST_DATA TestData;
>>>
>>>    CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>>>    // ...
>>>  }
>>>
>>> Thanks
>>> Laszlo
>>>
>>>>
>>>> Best regards,
>>>>
>>>> Mike
>>>>
>>>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
>>>> Sent: Tuesday, October 6, 2020 5:28 PM
>>>> To: devel@edk2.groups.io
>>>> Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
>>>>
>>>> Ive worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
>>>> Commits · corthon/edk2 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommits%2Fvar_policy_dev_submission_v8&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=XSNAxTGdbKGlrPLV%2BqWVNEAQjyyeSoKQ8zcfG1%2B4W1s%3D&amp;reserved=0>
>>>>
>>>> I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
>>>> TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210 (github.com)<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fcorthon%2Fedk2%2Fcommit%2F4ce52108b3e1bcb2ba78995be94c3949fe647eda&amp;data=04%7C01%7Cbret.barkelew%40microsoft.com%7C8286accbe81740bde8cc08d86ad8c8ae%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637376826604910138%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=46MZeja1WTnq44vSZOwJQ%2BAs61TbdFQtFfyj7wgm5mY%3D&amp;reserved=0>
>>>>
>>>> However, theres one that I just cant get past and I would like to take it up with the community. I dont think that UnitTests should have to deal with the \x1ccant initialize variables in declaration check. Almost none of the solutions that I tested worked, and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
>>>>
>>>>  *   They were annoying to write ===> fewer tests.
>>>>  *   They moved even more of the test case data away from the test ===> harder to read tests.
>>>>
>>>> I would like to move for an exception for unit tests (or at least host-based unit tests), but I dont know how to accomplish that from a technical standpoint.
>>>>
>>>> Thoughts?
>>>>
>>>> - Bret
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> 
>>>
>>>
>>
> 

Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest

[Michael D Kinney]

Hi Laszlo,

Option #3 looks really good to me.  And I agree that ECC should not generate 
an error if a STATIC CONST local variable is initialized.

Thanks,

Mike

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Wednesday, October 7, 2020 6:43 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> Subject: Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
> 
> On 10/07/20 03:46, Michael D Kinney wrote:
> >
> > Bret,
> >
> > Initializing variable in declaration for structures and arrays
> > introduces use of intrinsics.  Since it is possible for unit test
> > sources to be used for both host and target tests, I recommend we
> > continue to follow the EDK II coding style for unit tests to support
> > maximum compatibility and code reuse.
> >
> > Using a module global variable with initializers instead of
> > initializing a local declaration is the same amount of work, so I do
> > not believe that will result in fewer tests.
> >
> > I agree it is useful to have the test data next to the test code. This
> > can be accomplished by breaking up into more files so the test data is
> > immediately above the test function the test data is used.  Does ECC
> > raise an error if a module global is placed between 2 functions?  A
> > 2nd approach to put the module global immediately above the test
> > function the test data is used.
> 
> Consider the following example structure type, for the sake of
> discussion:
> 
>   typedef struct {
>     UINT32 Value;
>   } TEST_DATA;
> 
> 
> * Case#1: block scope, automatic storage duration
> 
>   EFI_STATUS
>   FoobarTest (
>     VOID
>     )
>   {
>     TEST_DATA TestData = { 42 };
>     // ...
>   }
> 
> Problem: uses intrinsics.
> 
> 
> * Case#2: file scope, static storage duration.
> 
>   STATIC CONST TEST_DATA mTestData = { 42 };
> 
>   EFI_STATUS
>   FoobarTest (
>     VOID
>     )
>   {
>     // ...
>   }
> 
> Problem: either "mTestData" is textually far from FoobarTest(), or -- if
> we keep them close to each other -- we mix variable definitions with
> function definitions, at file scope.
> 
> 
> * Case #3: block scope, static storage duration.
> 
>   EFI_STATUS
>   FoobarTest (
>     VOID
>     )
>   {
>     STATIC CONST TEST_DATA TestData = { 42 };
>     // ...
>   }
> 
> Problem: there should be none. Does not involve intrinsics, and the
> object definition is part of the function's scope.
> 
> 
> If ECC does not recognize case#3 as valid, then that is an *ECC bug*.
> 
> ECC has no reason to prevent case#3, as case#3 does not involve
> intrinsics, and is a generally valid and useful C language construct (it
> combines the life cycle of case#2 with the visibility of case#1).
> 
> Again, if ECC rejects case#3, that's *definitely* a bug in ECC, and we
> should fix it first. Given that ECC includes a full-blown C language
> parser, the fix should not be too difficult -- check if the declaration
> has the "static" storage-class specifier.
> 
> ... In fact, I think that purely CONST-qualifying TestData might suffice
> for shutting up ECC. See the following in
> "BaseTools/Source/Python/Ecc/c.py", method
> "CheckFuncLayoutLocalVariable":
> 
> >         for Result in ResultSet:
> >             if len(Result[1]) > 0 and 'CONST' not in Result[3]:
> >                 PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_VARIABLE, 'Variable Name: %s' % Result[0], FileTable,
> Result[2])
> 
> So case#3 should work through that avenue already, because case#3 has
> CONST *too*.
> 
> Now, in case#3, if "TestData" needs to undergo modifications, and so
> CONST is not immediately desirable, that's solvable:
> 
>   EFI_STATUS
>   FoobarTest (
>     VOID
>     )
>   {
>     STATIC CONST TEST_DATA TestDataTemplate = { 42 };
>     TEST_DATA TestData;
> 
>     CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA));
>     // ...
>   }
> 
> Thanks
> Laszlo
> 
> >
> > Best regards,
> >
> > Mike
> >
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Bret Barkelew via groups.io
> > Sent: Tuesday, October 6, 2020 5:28 PM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest
> >
> > I\x19ve worked through all the ECC issues with Variable Policy (AND the UnitTests) on this branch:
> > Commits · corthon/edk2 (github.com)<https://github.com/corthon/edk2/commits/var_policy_dev_submission_v8>
> >
> > I even wrote the Main() entry point lib that Laszlo suggested (it works rather nicely):
> > TEMP: Staging for HostTest entry point · corthon/edk2@4ce5210
> (github.com)<https://github.com/corthon/edk2/commit/4ce52108b3e1bcb2ba78995be94c3949fe647eda>
> >
> > However, there\x19s one that I just can\x19t get past and I would like to take it up with the community. I don\x19t think that UnitTests
> should have to deal with the \x1ccan\x19t initialize variables in declaration\x1d check. Almost none of the solutions that I tested worked,
> and the ones that did were too cumbersome. They failed on two key points that are important for test writing:
> >
> >   *   They were annoying to write ===> fewer tests.
> >   *   They moved even more of the test case data away from the test ===> harder to read tests.
> >
> > I would like to move for an exception for unit tests (or at least host-based unit tests), but I don\x19t know how to accomplish that
> from a technical standpoint.
> >
> > Thoughts?
> >
> > - Bret
> >
> >
> >
> >
> > 
> >
> >