From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) by mx.groups.io with SMTP id smtpd.web11.2431.1602080872602380580 for ; Wed, 07 Oct 2020 07:27:52 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=qBMyaU0C; spf=pass (domain: apple.com, ip: 17.171.2.72, mailfrom: afish@apple.com) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.42/8.16.0.42) with SMTP id 097EIMp5025387; Wed, 7 Oct 2020 07:27:51 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=content-type : content-transfer-encoding : from : mime-version : subject : date : message-id : references : cc : in-reply-to : to; s=20180706; bh=SvTKyZ0rk/xHs1Yix/ny+w7nfDxU2kokAHhBi9e2gXw=; b=qBMyaU0C0kX8s2gaUZmS6CHINEjgmCSA8neTgmOYlVCxUVLoq/9n3cRj0SVX4cGBzzzV RuLDU10Av0P7cFTCWSY78F2VogVDbTFWqSAB5d31Owgu8p3F3Etxsgpf37dN4Aihjx7w 8Lq3Itvu1anYK5JdEtD34FlfBR1xFPVjnwnrGPL3RJCz4PpKgkWSXuwmPh0AOCq0AV21 Jy3y/nSxmz3pAvbFAj8BHi5FGHEIpFT4kAztcfhT6H8uqPFQ4AVgafp/haM1YvC0ig04 N1tJMmoUkZc1sQJjT+xhW+HIsxAehB7uuCmNUjhYirSZ5nh/L8jU9J+rADVd3CYYZ6ox 9Q== Received: from rn-mailsvcp-mta-lapp03.rno.apple.com (rn-mailsvcp-mta-lapp03.rno.apple.com [10.225.203.151]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 33xr9undds-5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 07 Oct 2020 07:27:51 -0700 Received: from rn-mailsvcp-mmp-lapp01.rno.apple.com (rn-mailsvcp-mmp-lapp01.rno.apple.com [17.179.253.14]) by rn-mailsvcp-mta-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPS id <0QHU00PGV5IEDWF0@rn-mailsvcp-mta-lapp03.rno.apple.com>; Wed, 07 Oct 2020 07:27:50 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp01.rno.apple.com by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) id <0QHU00Q004VRK700@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Wed, 07 Oct 2020 07:27:50 -0700 (PDT) X-Va-A: X-Va-T-CD: 076955f3becce2df77d006c843a15678 X-Va-E-CD: 0fefce96c10172c12b3925d379e0b338 X-Va-R-CD: 9e5f5d3e974d58ebc9c43c2c1f4a824f X-Va-CD: 0 X-Va-ID: 388ef84f-d84f-47a3-b8ae-6523f3224da8 X-V-A: X-V-T-CD: 076955f3becce2df77d006c843a15678 X-V-E-CD: 0fefce96c10172c12b3925d379e0b338 X-V-R-CD: 9e5f5d3e974d58ebc9c43c2c1f4a824f X-V-CD: 0 X-V-ID: f239e957-cfd6-48b1-a390-ef40db93bafa X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-10-07_09:2020-10-06,2020-10-07 signatures=0 Received: from [10.104.155.56] (unknown [10.104.155.56]) by rn-mailsvcp-mmp-lapp01.rno.apple.com (Oracle Communications Messaging Server 8.1.0.6.20200729 64bit (built Jul 29 2020)) with ESMTPSA id <0QHU00TJM5ICNL00@rn-mailsvcp-mmp-lapp01.rno.apple.com>; Wed, 07 Oct 2020 07:27:50 -0700 (PDT) From: "Andrew Fish" MIME-version: 1.0 (1.0) Subject: Re: [edk2-devel] VariablePolicy: Final Changes Thread 2 - ECC & UnitTest Date: Wed, 07 Oct 2020 07:27:47 -0700 Message-id: <6AB2D4A8-B715-4755-A2A0-804BBC292AA3@apple.com> References: <742c37aa-59a8-ac80-ee61-5173be35afea@redhat.com> Cc: michael.d.kinney@intel.com, Bret Barkelew In-reply-to: <742c37aa-59a8-ac80-ee61-5173be35afea@redhat.com> To: devel@edk2.groups.io, lersek@redhat.com X-Mailer: iPhone Mail (18A393) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.235,18.0.687 definitions=2020-10-07_09:2020-10-06,2020-10-07 signatures=0 Content-type: text/plain; charset=utf-8 Content-transfer-encoding: quoted-printable 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 wrote: >=20 > =EF=BB=BFOn 10/07/20 03:46, Michael D Kinney wrote: >>=20 >> Bret, >>=20 >> 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. >>=20 >> 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. >>=20 >> 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. >=20 > Consider the following example structure type, for the sake of > discussion: >=20 > typedef struct { > UINT32 Value; > } TEST_DATA; >=20 >=20 > * Case#1: block scope, automatic storage duration >=20 > EFI_STATUS > FoobarTest ( > VOID > ) > { > TEST_DATA TestData =3D { 42 }; > // ... > } >=20 > Problem: uses intrinsics. >=20 >=20 > * Case#2: file scope, static storage duration. >=20 > STATIC CONST TEST_DATA mTestData =3D { 42 }; >=20 > EFI_STATUS > FoobarTest ( > VOID > ) > { > // ... > } >=20 > 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. >=20 >=20 > * Case #3: block scope, static storage duration. >=20 > EFI_STATUS > FoobarTest ( > VOID > ) > { > STATIC CONST TEST_DATA TestData =3D { 42 }; > // ... > } >=20 > Problem: there should be none. Does not involve intrinsics, and the > object definition is part of the function's scope. >=20 >=20 > If ECC does not recognize case#3 as valid, then that is an *ECC bug*. >=20 > 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). >=20 > 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. >=20 > ... 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": >=20 >> for Result in ResultSet: >> if len(Result[1]) > 0 and 'CONST' not in Result[3]: >> PrintErrorMsg(ERROR_C_FUNCTION_LAYOUT_CHECK_NO_INIT_OF_V= ARIABLE, 'Variable Name: %s' % Result[0], FileTable, Result[2]) >=20 > So case#3 should work through that avenue already, because case#3 has > CONST *too*. >=20 > Now, in case#3, if "TestData" needs to undergo modifications, and so > CONST is not immediately desirable, that's solvable: >=20 > EFI_STATUS > FoobarTest ( > VOID > ) > { > STATIC CONST TEST_DATA TestDataTemplate =3D { 42 }; > TEST_DATA TestData; >=20 > CopyMem (&TestData, TestDataTemplate, sizeof (TEST_DATA)); > // ... > } >=20 > Thanks > Laszlo >=20 >>=20 >> Best regards, >>=20 >> Mike >>=20 >> From: devel@edk2.groups.io On Behalf Of Bret Bar= kelew 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 & Un= itTest >>=20 >> I=19ve worked through all the ECC issues with Variable Policy (AND the = UnitTests) on this branch: >> Commits =C2=B7 corthon/edk2 (github.com) >>=20 >> I even wrote the Main() entry point lib that Laszlo suggested (it works= rather nicely): >> TEMP: Staging for HostTest entry point =C2=B7 corthon/edk2@4ce5210 (git= hub.com) >>=20 >> However, there=19s one that I just can=19t get past and I would like to= take it up with the community. I don=19t think that UnitTests should have = to deal with the =1Ccan=19t initialize variables in declaration=1D check. A= lmost none of the solutions that I tested worked, and the ones that did wer= e too cumbersome. They failed on two key points that are important for test= writing: >>=20 >> * They were annoying to write =3D=3D=3D> fewer tests. >> * They moved even more of the test case data away from the test =3D= =3D=3D> harder to read tests. >>=20 >> I would like to move for an exception for unit tests (or at least host-= based unit tests), but I don=19t know how to accomplish that from a technic= al standpoint. >>=20 >> Thoughts? >>=20 >> - Bret >>=20 >>=20 >>=20 >>=20 >>=20 >>=20 >>=20 >=20 >=20 >=20 >=20 >=20 >=20