From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.91089.1670977398674617052 for ; Tue, 13 Dec 2022 16:23:18 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=QO/uf39F; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 8F1CF20B878B; Tue, 13 Dec 2022 16:23:17 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 8F1CF20B878B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1670977398; bh=Hl0w0MmNwWwxRHAoiETfxSuCZwhgB7EeyLCWxHxtKc8=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=QO/uf39FlOJZjSse7H5RF1omcBsRHY1sH4xRV9hIwR0rQqJ7Am2kH04Niin6a4rZG /dj2mbLMWuDiinEcKJvBMXsW4MdRXLqKRsCf7m9BQ2cgIaZNVTubUxfyeAwvNltnma DDE7qZUG+LJEY5FsAtklXgu6tJUJoZP2YbBN8xJM= Message-ID: Date: Tue, 13 Dec 2022 19:23:16 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [edk2-devel] [PATCH] UnitTestFrameworkPkg: Modify APIs in UnitTestPersistenceLib To: devel@edk2.groups.io, zhiguang.liu@intel.com, "Kinney, Michael D" , Sean Brogan Cc: "Ni, Ray" References: <172E1CF5498F8F29.13790@groups.io> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Reviewed-by: Michael Kubacki On 12/12/2022 2:32 AM, Zhiguang Liu wrote: > Hi UnitTestFrameworkPkg Maintainers and reviewers, > > Could you help review this patch? > > Thanks > Zhiguang > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of >> Zhiguang Liu >> Sent: Tuesday, December 6, 2022 1:26 PM >> To: devel@edk2.groups.io >> Cc: Liu, Zhiguang ; Kinney, Michael D >> ; Michael Kubacki >> ; Sean Brogan >> ; Ni, Ray >> Subject: [edk2-devel] [PATCH] UnitTestFrameworkPkg: Modify APIs in >> UnitTestPersistenceLib >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4183 >> >> UnitTestPersistenceLib now consumes private struct definition. >> Modify APIs in UnitTestPersistenceLib to make it easy to become a public >> library. >> >> Cc: Michael D Kinney >> Cc: Michael Kubacki >> Cc: Sean Brogan >> Cc: Ray Ni >> Signed-off-by: Zhiguang Liu >> --- >> .../Include/Library/UnitTestPersistenceLib.h | 14 +++++++---- >> .../Library/UnitTestLib/UnitTestLib.c | 6 +++-- >> .../UnitTestPersistenceLibNull.c | 11 ++++++--- >> .../UnitTestPersistenceLibSimpleFileSystem.c | 23 ++++++++++++------- >> 4 files changed, 36 insertions(+), 18 deletions(-) >> >> diff --git a/UnitTestFrameworkPkg/Include/Library/UnitTestPersistenceLib.h >> b/UnitTestFrameworkPkg/Include/Library/UnitTestPersistenceLib.h >> index be29e079ec..5543b79a0d 100644 >> --- a/UnitTestFrameworkPkg/Include/Library/UnitTestPersistenceLib.h >> +++ b/UnitTestFrameworkPkg/Include/Library/UnitTestPersistenceLib.h >> @@ -4,7 +4,7 @@ >> (eg. a reboot-based test). >> >> Copyright (c) Microsoft Corporation.
>> - Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.
>> + Copyright (c) 2019 - 2022, Intel Corporation. All rights >> + reserved.
>> SPDX-License-Identifier: BSD-2-Clause-Patent >> >> **/ >> @@ -12,7 +12,7 @@ >> #ifndef _UNIT_TEST_PERSISTENCE_LIB_H_ >> #define _UNIT_TEST_PERSISTENCE_LIB_H_ >> >> -#include >> +#include >> >> #define UNIT_TEST_PERSISTENCE_LIB_VERSION 1 >> >> @@ -40,6 +40,7 @@ DoesCacheExist ( >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> @param[in] SaveData A pointer to the buffer containing the serialized >> framework internal state. >> + @param[in] SaveStateSize The size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data is persisted and the test can be safely quit. >> @retval Others Data is not persisted and test cannot be resumed >> upon exit. >> @@ -49,7 +50,8 @@ EFI_STATUS >> EFIAPI >> SaveUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - IN UNIT_TEST_SAVE_HEADER *SaveData >> + IN VOID *SaveData, >> + IN UINTN SaveStateSize >> ); >> >> /** >> @@ -57,8 +59,9 @@ SaveUnitTestCache ( >> Will allocate a buffer to hold the loaded data. >> >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> - @param[in] SaveData A pointer pointer that will be updated with the >> address >> + @param[out] SaveData A pointer pointer that will be updated with >> the address >> of the loaded data buffer. >> + @param[out] SaveStateSize Return the size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data has been loaded successfully and SaveData >> is updated >> with a pointer to the buffer. >> @@ -70,7 +73,8 @@ EFI_STATUS >> EFIAPI >> LoadUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - OUT UNIT_TEST_SAVE_HEADER **SaveData >> + OUT VOID **SaveData, >> + OUT UINTN *SaveStateSize >> ); >> >> #endif >> diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c >> b/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c >> index 64d5880783..5b442ed122 100644 >> --- a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c >> +++ b/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c >> @@ -2,6 +2,7 @@ >> Implement UnitTestLib >> >> Copyright (c) Microsoft Corporation. >> + Copyright (c) 2022, Intel Corporation. All rights reserved.
>> SPDX-License-Identifier: BSD-2-Clause-Patent **/ >> >> @@ -210,6 +211,7 @@ InitUnitTestFramework ( >> EFI_STATUS Status; >> UNIT_TEST_FRAMEWORK_HANDLE NewFrameworkHandle; >> UNIT_TEST_FRAMEWORK *NewFramework; >> + UINTN SaveStateSize; >> >> Status = EFI_SUCCESS; >> NewFramework = NULL; >> @@ -267,7 +269,7 @@ InitUnitTestFramework ( >> // If there is a persisted context, load it now. >> // >> if (DoesCacheExist (NewFrameworkHandle)) { >> - Status = LoadUnitTestCache (NewFrameworkHandle, >> (UNIT_TEST_SAVE_HEADER **)(&NewFramework->SavedState)); >> + Status = LoadUnitTestCache (NewFrameworkHandle, (VOID >> + **)(&NewFramework->SavedState), &SaveStateSize); >> if (EFI_ERROR (Status)) { >> // >> // Don't actually report it as an error, but emit a warning. >> @@ -852,7 +854,7 @@ SaveFrameworkState ( >> // >> // All that should be left to do is save it using the associated persistence lib. >> // >> - Status = SaveUnitTestCache (FrameworkHandle, Header); >> + Status = SaveUnitTestCache (FrameworkHandle, Header, >> + Header->SaveStateSize); >> if (EFI_ERROR (Status)) { >> DEBUG ((DEBUG_ERROR, "%a - Could not save state! %r\n", >> __FUNCTION__, Status)); >> Status = EFI_DEVICE_ERROR; >> diff --git >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersis >> tenceLibNull.c >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersis >> tenceLibNull.c >> index e28327652e..abb24cff98 100644 >> --- >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersis >> tenceLibNull.c >> +++ >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPe >> +++ rsistenceLibNull.c >> @@ -2,6 +2,7 @@ >> This is an instance of the Unit Test Persistence Lib that does nothing. >> >> Copyright (c) Microsoft Corporation.
>> + Copyright (c) 2022, Intel Corporation. All rights reserved.
>> SPDX-License-Identifier: BSD-2-Clause-Patent **/ >> >> @@ -35,6 +36,7 @@ DoesCacheExist ( >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> @param[in] SaveData A pointer to the buffer containing the serialized >> framework internal state. >> + @param[in] SaveStateSize The size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data is persisted and the test can be safely quit. >> @retval Others Data is not persisted and test cannot be resumed upon >> exit. >> @@ -44,7 +46,8 @@ EFI_STATUS >> EFIAPI >> SaveUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - IN UNIT_TEST_SAVE_HEADER *SaveData >> + IN VOID *SaveData, >> + IN UINTN SaveStateSize >> ) >> { >> return EFI_UNSUPPORTED; >> @@ -55,8 +58,9 @@ SaveUnitTestCache ( >> Will allocate a buffer to hold the loaded data. >> >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> - @param[in] SaveData A pointer pointer that will be updated with the >> address >> + @param[out] SaveData A pointer pointer that will be updated with the >> address >> of the loaded data buffer. >> + @param[out] SaveStateSize Return the size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data has been loaded successfully and SaveData is >> updated >> with a pointer to the buffer. >> @@ -68,7 +72,8 @@ EFI_STATUS >> EFIAPI >> LoadUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - OUT UNIT_TEST_SAVE_HEADER **SaveData >> + OUT VOID **SaveData, >> + OUT UINTN *SaveStateSize >> ) >> { >> return EFI_UNSUPPORTED; >> diff --git >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/ >> UnitTestPersistenceLibSimpleFileSystem.c >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/ >> UnitTestPersistenceLibSimpleFileSystem.c >> index ed4a7d1615..d7a62145da 100644 >> --- >> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/ >> UnitTestPersistenceLibSimpleFileSystem.c >> +++ >> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSyste >> +++ m/UnitTestPersistenceLibSimpleFileSystem.c >> @@ -4,6 +4,7 @@ >> version of the internal test state in case the test needs to quit and restore. >> >> Copyright (c) Microsoft Corporation.
>> + Copyright (c) 2022, Intel Corporation. All rights reserved.
>> SPDX-License-Identifier: BSD-2-Clause-Patent **/ >> >> @@ -16,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> >> #define CACHE_FILE_SUFFIX L"_Cache.dat" >> >> @@ -213,6 +215,7 @@ DoesCacheExist ( >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> @param[in] SaveData A pointer to the buffer containing the serialized >> framework internal state. >> + @param[in] SaveStateSize The size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data is persisted and the test can be safely quit. >> @retval Others Data is not persisted and test cannot be resumed upon >> exit. >> @@ -222,7 +225,8 @@ EFI_STATUS >> EFIAPI >> SaveUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - IN UNIT_TEST_SAVE_HEADER *SaveData >> + IN VOID *SaveData, >> + IN UINTN SaveStateSize >> ) >> { >> EFI_DEVICE_PATH_PROTOCOL *FileDevicePath; @@ -280,7 +284,7 @@ >> SaveUnitTestCache ( >> // >> // Write the data to the file. >> // >> - WriteCount = SaveData->SaveStateSize; >> + WriteCount = SaveStateSize; >> DEBUG ((DEBUG_INFO, "%a - Writing %d bytes to file...\n", __FUNCTION__, >> WriteCount)); >> Status = ShellWriteFile ( >> FileHandle, >> @@ -288,7 +292,7 @@ SaveUnitTestCache ( >> SaveData >> ); >> >> - if (EFI_ERROR (Status) || (WriteCount != SaveData->SaveStateSize)) { >> + if (EFI_ERROR (Status) || (WriteCount != SaveStateSize)) { >> DEBUG ((DEBUG_ERROR, "%a - Writing to file failed! %r\n", >> __FUNCTION__, Status)); >> } else { >> DEBUG ((DEBUG_INFO, "%a - SUCCESS!\n", __FUNCTION__)); @@ -312,8 >> +316,9 @@ Exit: >> Will allocate a buffer to hold the loaded data. >> >> @param[in] FrameworkHandle A pointer to the framework that is being >> persisted. >> - @param[in] SaveData A pointer pointer that will be updated with the >> address >> + @param[out] SaveData A pointer pointer that will be updated with the >> address >> of the loaded data buffer. >> + @param[out] SaveStateSize Return the size of SaveData in bytes. >> >> @retval EFI_SUCCESS Data has been loaded successfully and SaveData is >> updated >> with a pointer to the buffer. >> @@ -325,7 +330,8 @@ EFI_STATUS >> EFIAPI >> LoadUnitTestCache ( >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle, >> - OUT UNIT_TEST_SAVE_HEADER **SaveData >> + OUT VOID **SaveData, >> + OUT UINTN *SaveStateSize >> ) >> { >> EFI_STATUS Status; >> @@ -334,7 +340,7 @@ LoadUnitTestCache ( >> BOOLEAN IsFileOpened; >> UINT64 LargeFileSize; >> UINTN FileSize; >> - UNIT_TEST_SAVE_HEADER *Buffer; >> + VOID *Buffer; >> >> IsFileOpened = FALSE; >> Buffer = NULL; >> @@ -380,8 +386,9 @@ LoadUnitTestCache ( >> // >> // Now that we know the size, let's allocated a buffer to hold the contents. >> // >> - FileSize = (UINTN)LargeFileSize; // You know what... if it's too large, this lib >> don't care. >> - Buffer = AllocatePool (FileSize); >> + FileSize = (UINTN)LargeFileSize; // You know what... if it's too large, this >> lib don't care. >> + *SaveStateSize = FileSize; >> + Buffer = AllocatePool (FileSize); >> if (Buffer == NULL) { >> DEBUG ((DEBUG_ERROR, "%a - Failed to allocate a pool to hold the file >> contents! %r\n", __FUNCTION__, Status)); >> Status = EFI_OUT_OF_RESOURCES; >> -- >> 2.31.1.windows.1 >> >> >> >> >> > > > > >