* [PATCH v1 0/1] Add support for running shell test application in an immutable volume
@ 2023-05-31 19:13 Kun Qin
2023-05-31 19:13 ` [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive Kun Qin
0 siblings, 1 reply; 7+ messages in thread
From: Kun Qin @ 2023-05-31 19:13 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Michael Kubacki, Sean Brogan
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
Current implementation of UnitTestFrameworkPkg for shell-based unit test
will save the unit test cache to the same file system as the test
application itself. This works as long as the test application is on a
writable volume, such as USB or EFI partition.
However, sometimes the test application would live inside the firmware
volume because it was integrated into the UEFI image and published
through drivers such as "FvSimpleFileSystem". In this case, attempting to
write cache file to the file system will fail and thus unable to run the
tests.
The included change will write the test cache to the path where this unit
test is invoked. i.e. test is on FS0, which is a FV file system, but FS1
is a usb file system. The traditional flow of running this test in shell
will be "fs0:", "test.efi", which will fail to write to "fs0:". The
updated flow is to navigate shell to "fs1:", then execute the test with
"fs0:test.efi". This way the cache file will be saved to "fs1:" properly.
This change will have no impact on existing users who already run shell
based tests from writeable file systesms, such as USB, FAT on NVMe.
Patch v1 branch: https://github.com/kuqin12/edk2/tree/unit_test_fv_v1.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
kuqin12 (1):
UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to
Drive
UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
1 file changed, 43 insertions(+), 38 deletions(-)
--
2.40.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-05-31 19:13 [PATCH v1 0/1] Add support for running shell test application in an immutable volume Kun Qin
@ 2023-05-31 19:13 ` Kun Qin
2023-05-31 20:39 ` Michael D Kinney
0 siblings, 1 reply; 7+ messages in thread
From: Kun Qin @ 2023-05-31 19:13 UTC (permalink / raw)
To: devel; +Cc: Sean Brogan, Michael Kubacki, Michael D Kinney
From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
Current implementation of UnitTestFrameworkPkg for shell-based unit test
will save the unit test cache to the same volume as the test application
itself. This works as long as the test application is on a writable
volume, such as USB or EFI partition.
Instead of saving the files to the same file system of unit test
application, this change will save the cache file to the path where the
user ran this test application.
This change was tested on proprietary physical hardware platforms and
QEMU based virtual platform.
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Kubacki <mikuback@linux.microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Kun Qin <kuqin12@gmail.com>
---
UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
1 file changed, 43 insertions(+), 38 deletions(-)
diff --git a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
index b59991683f48..d4181808b2be 100644
--- a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
+++ b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
@@ -22,7 +22,7 @@
#define CACHE_FILE_SUFFIX L"_Cache.dat"
/**
- Generate the device path to the cache file.
+ Generate the file name and path to the cache file.
@param[in] FrameworkHandle A pointer to the framework that is being persisted.
@@ -31,8 +31,8 @@
**/
STATIC
-EFI_DEVICE_PATH_PROTOCOL *
-GetCacheFileDevicePath (
+CHAR16 *
+GetCacheFileName (
IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
)
{
@@ -85,7 +85,12 @@ GetCacheFileDevicePath (
// PathCleanUpDirectories (FileNameCopy);
// if (PathRemoveLastItem (FileNameCopy)) {
//
- AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, TRUE); // NOTE: This must be freed.
+ AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, TRUE); // NOTE: This must be freed.
+ if (AppPath == NULL) {
+ DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to text.\n", __func__));
+ goto Exit;
+ }
+
DirectorySlashOffset = StrLen (AppPath);
//
// Make sure we didn't get any weird data.
@@ -148,15 +153,15 @@ Exit:
FreePool (AppPath);
}
- if (CacheFilePath != NULL) {
- FreePool (CacheFilePath);
+ if (CacheFileDevicePath != NULL) {
+ FreePool (CacheFileDevicePath);
}
if (TestName != NULL) {
FreePool (TestName);
}
- return CacheFileDevicePath;
+ return CacheFilePath;
}
/**
@@ -175,21 +180,21 @@ DoesCacheExist (
IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
)
{
- EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
- EFI_STATUS Status;
- SHELL_FILE_HANDLE FileHandle;
+ CHAR16 *FileName;
+ EFI_STATUS Status;
+ SHELL_FILE_HANDLE FileHandle;
//
// NOTE: This devpath is allocated and must be freed.
//
- FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
+ FileName = GetCacheFileName (FrameworkHandle);
//
// Check to see whether the file exists. If the file can be opened for
// reading, it exists. Otherwise, probably not.
//
- Status = ShellOpenFileByDevicePath (
- &FileDevicePath,
+ Status = ShellOpenFileByName (
+ FileName,
&FileHandle,
EFI_FILE_MODE_READ,
0
@@ -198,8 +203,8 @@ DoesCacheExist (
ShellCloseFile (&FileHandle);
}
- if (FileDevicePath != NULL) {
- FreePool (FileDevicePath);
+ if (FileName != NULL) {
+ FreePool (FileName);
}
DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR (Status)));
@@ -229,10 +234,10 @@ SaveUnitTestCache (
IN UINTN SaveStateSize
)
{
- EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
- EFI_STATUS Status;
- SHELL_FILE_HANDLE FileHandle;
- UINTN WriteCount;
+ CHAR16 *FileName;
+ EFI_STATUS Status;
+ SHELL_FILE_HANDLE FileHandle;
+ UINTN WriteCount;
//
// Check the inputs for sanity.
@@ -245,13 +250,13 @@ SaveUnitTestCache (
// Determine the path for the cache file.
// NOTE: This devpath is allocated and must be freed.
//
- FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
+ FileName = GetCacheFileName (FrameworkHandle);
//
// First lets open the file if it exists so we can delete it...This is the work around for truncation
//
- Status = ShellOpenFileByDevicePath (
- &FileDevicePath,
+ Status = ShellOpenFileByName (
+ FileName,
&FileHandle,
(EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
0
@@ -270,8 +275,8 @@ SaveUnitTestCache (
//
// Now that we know the path to the file... let's open it for writing.
//
- Status = ShellOpenFileByDevicePath (
- &FileDevicePath,
+ Status = ShellOpenFileByName (
+ FileName,
&FileHandle,
(EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE | EFI_FILE_MODE_CREATE),
0
@@ -304,8 +309,8 @@ SaveUnitTestCache (
ShellCloseFile (&FileHandle);
Exit:
- if (FileDevicePath != NULL) {
- FreePool (FileDevicePath);
+ if (FileName != NULL) {
+ FreePool (FileName);
}
return Status;
@@ -334,13 +339,13 @@ LoadUnitTestCache (
OUT UINTN *SaveStateSize
)
{
- EFI_STATUS Status;
- EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
- SHELL_FILE_HANDLE FileHandle;
- BOOLEAN IsFileOpened;
- UINT64 LargeFileSize;
- UINTN FileSize;
- VOID *Buffer;
+ EFI_STATUS Status;
+ CHAR16 *FileName;
+ SHELL_FILE_HANDLE FileHandle;
+ BOOLEAN IsFileOpened;
+ UINT64 LargeFileSize;
+ UINTN FileSize;
+ VOID *Buffer;
IsFileOpened = FALSE;
Buffer = NULL;
@@ -356,13 +361,13 @@ LoadUnitTestCache (
// Determine the path for the cache file.
// NOTE: This devpath is allocated and must be freed.
//
- FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
+ FileName = GetCacheFileName (FrameworkHandle);
//
// Now that we know the path to the file... let's open it for writing.
//
- Status = ShellOpenFileByDevicePath (
- &FileDevicePath,
+ Status = ShellOpenFileByName (
+ FileName,
&FileHandle,
EFI_FILE_MODE_READ,
0
@@ -407,8 +412,8 @@ Exit:
//
// Free allocated buffers
//
- if (FileDevicePath != NULL) {
- FreePool (FileDevicePath);
+ if (FileName != NULL) {
+ FreePool (FileName);
}
if (IsFileOpened) {
--
2.40.1.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-05-31 19:13 ` [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive Kun Qin
@ 2023-05-31 20:39 ` Michael D Kinney
2023-06-01 15:50 ` Kun Qin
0 siblings, 1 reply; 7+ messages in thread
From: Michael D Kinney @ 2023-05-31 20:39 UTC (permalink / raw)
To: Kun Qin, devel@edk2.groups.io
Cc: Sean Brogan, Michael Kubacki, Kinney, Michael D
Would it be more flexible for unit tests apps to support an optional command line arg
to specify the volume to write results?
Mike
> -----Original Message-----
> From: Kun Qin <kuqin12@gmail.com>
> Sent: Wednesday, May 31, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save
> Unit Test Cache to Drive
>
> From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
>
> Current implementation of UnitTestFrameworkPkg for shell-based unit test
> will save the unit test cache to the same volume as the test application
> itself. This works as long as the test application is on a writable
> volume, such as USB or EFI partition.
>
> Instead of saving the files to the same file system of unit test
> application, this change will save the cache file to the path where the
> user ran this test application.
>
> This change was tested on proprietary physical hardware platforms and
> QEMU based virtual platform.
>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> ---
>
> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes
> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
> 1 file changed, 43 insertions(+), 38 deletions(-)
>
> diff --git
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> estPersistenceLibSimpleFileSystem.c
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> estPersistenceLibSimpleFileSystem.c
> index b59991683f48..d4181808b2be 100644
> ---
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> estPersistenceLibSimpleFileSystem.c
> +++
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> estPersistenceLibSimpleFileSystem.c
> @@ -22,7 +22,7 @@
> #define CACHE_FILE_SUFFIX L"_Cache.dat"
>
>
>
> /**
>
> - Generate the device path to the cache file.
>
> + Generate the file name and path to the cache file.
>
>
>
> @param[in] FrameworkHandle A pointer to the framework that is being
> persisted.
>
>
>
> @@ -31,8 +31,8 @@
>
>
> **/
>
> STATIC
>
> -EFI_DEVICE_PATH_PROTOCOL *
>
> -GetCacheFileDevicePath (
>
> +CHAR16 *
>
> +GetCacheFileName (
>
> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>
> )
>
> {
>
> @@ -85,7 +85,12 @@ GetCacheFileDevicePath (
> // PathCleanUpDirectories (FileNameCopy);
>
> // if (PathRemoveLastItem (FileNameCopy)) {
>
> //
>
> - AppPath = ConvertDevicePathToText (LoadedImage->FilePath,
> TRUE, TRUE); // NOTE: This must be freed.
>
> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, TRUE);
> // NOTE: This must be freed.
>
> + if (AppPath == NULL) {
>
> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to text.\n",
> __func__));
>
> + goto Exit;
>
> + }
>
> +
>
> DirectorySlashOffset = StrLen (AppPath);
>
> //
>
> // Make sure we didn't get any weird data.
>
> @@ -148,15 +153,15 @@ Exit:
> FreePool (AppPath);
>
> }
>
>
>
> - if (CacheFilePath != NULL) {
>
> - FreePool (CacheFilePath);
>
> + if (CacheFileDevicePath != NULL) {
>
> + FreePool (CacheFileDevicePath);
>
> }
>
>
>
> if (TestName != NULL) {
>
> FreePool (TestName);
>
> }
>
>
>
> - return CacheFileDevicePath;
>
> + return CacheFilePath;
>
> }
>
>
>
> /**
>
> @@ -175,21 +180,21 @@ DoesCacheExist (
> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>
> )
>
> {
>
> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>
> - EFI_STATUS Status;
>
> - SHELL_FILE_HANDLE FileHandle;
>
> + CHAR16 *FileName;
>
> + EFI_STATUS Status;
>
> + SHELL_FILE_HANDLE FileHandle;
>
>
>
> //
>
> // NOTE: This devpath is allocated and must be freed.
>
> //
>
> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>
> + FileName = GetCacheFileName (FrameworkHandle);
>
>
>
> //
>
> // Check to see whether the file exists. If the file can be opened for
>
> // reading, it exists. Otherwise, probably not.
>
> //
>
> - Status = ShellOpenFileByDevicePath (
>
> - &FileDevicePath,
>
> + Status = ShellOpenFileByName (
>
> + FileName,
>
> &FileHandle,
>
> EFI_FILE_MODE_READ,
>
> 0
>
> @@ -198,8 +203,8 @@ DoesCacheExist (
> ShellCloseFile (&FileHandle);
>
> }
>
>
>
> - if (FileDevicePath != NULL) {
>
> - FreePool (FileDevicePath);
>
> + if (FileName != NULL) {
>
> + FreePool (FileName);
>
> }
>
>
>
> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR
> (Status)));
>
> @@ -229,10 +234,10 @@ SaveUnitTestCache (
> IN UINTN SaveStateSize
>
> )
>
> {
>
> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>
> - EFI_STATUS Status;
>
> - SHELL_FILE_HANDLE FileHandle;
>
> - UINTN WriteCount;
>
> + CHAR16 *FileName;
>
> + EFI_STATUS Status;
>
> + SHELL_FILE_HANDLE FileHandle;
>
> + UINTN WriteCount;
>
>
>
> //
>
> // Check the inputs for sanity.
>
> @@ -245,13 +250,13 @@ SaveUnitTestCache (
> // Determine the path for the cache file.
>
> // NOTE: This devpath is allocated and must be freed.
>
> //
>
> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>
> + FileName = GetCacheFileName (FrameworkHandle);
>
>
>
> //
>
> // First lets open the file if it exists so we can delete it...This is
> the work around for truncation
>
> //
>
> - Status = ShellOpenFileByDevicePath (
>
> - &FileDevicePath,
>
> + Status = ShellOpenFileByName (
>
> + FileName,
>
> &FileHandle,
>
> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
>
> 0
>
> @@ -270,8 +275,8 @@ SaveUnitTestCache (
> //
>
> // Now that we know the path to the file... let's open it for writing.
>
> //
>
> - Status = ShellOpenFileByDevicePath (
>
> - &FileDevicePath,
>
> + Status = ShellOpenFileByName (
>
> + FileName,
>
> &FileHandle,
>
> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
> EFI_FILE_MODE_CREATE),
>
> 0
>
> @@ -304,8 +309,8 @@ SaveUnitTestCache (
> ShellCloseFile (&FileHandle);
>
>
>
> Exit:
>
> - if (FileDevicePath != NULL) {
>
> - FreePool (FileDevicePath);
>
> + if (FileName != NULL) {
>
> + FreePool (FileName);
>
> }
>
>
>
> return Status;
>
> @@ -334,13 +339,13 @@ LoadUnitTestCache (
> OUT UINTN *SaveStateSize
>
> )
>
> {
>
> - EFI_STATUS Status;
>
> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>
> - SHELL_FILE_HANDLE FileHandle;
>
> - BOOLEAN IsFileOpened;
>
> - UINT64 LargeFileSize;
>
> - UINTN FileSize;
>
> - VOID *Buffer;
>
> + EFI_STATUS Status;
>
> + CHAR16 *FileName;
>
> + SHELL_FILE_HANDLE FileHandle;
>
> + BOOLEAN IsFileOpened;
>
> + UINT64 LargeFileSize;
>
> + UINTN FileSize;
>
> + VOID *Buffer;
>
>
>
> IsFileOpened = FALSE;
>
> Buffer = NULL;
>
> @@ -356,13 +361,13 @@ LoadUnitTestCache (
> // Determine the path for the cache file.
>
> // NOTE: This devpath is allocated and must be freed.
>
> //
>
> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>
> + FileName = GetCacheFileName (FrameworkHandle);
>
>
>
> //
>
> // Now that we know the path to the file... let's open it for writing.
>
> //
>
> - Status = ShellOpenFileByDevicePath (
>
> - &FileDevicePath,
>
> + Status = ShellOpenFileByName (
>
> + FileName,
>
> &FileHandle,
>
> EFI_FILE_MODE_READ,
>
> 0
>
> @@ -407,8 +412,8 @@ Exit:
> //
>
> // Free allocated buffers
>
> //
>
> - if (FileDevicePath != NULL) {
>
> - FreePool (FileDevicePath);
>
> + if (FileName != NULL) {
>
> + FreePool (FileName);
>
> }
>
>
>
> if (IsFileOpened) {
>
> --
> 2.40.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-05-31 20:39 ` Michael D Kinney
@ 2023-06-01 15:50 ` Kun Qin
2023-06-01 16:48 ` Michael D Kinney
0 siblings, 1 reply; 7+ messages in thread
From: Kun Qin @ 2023-06-01 15:50 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io; +Cc: Sean Brogan, Michael Kubacki
Hi Mike,
Yes, that would indeed serve the purpose. Could you please show me a
pointer on how to add that
commonly? I would be open to change the code the other way.
The reason I would like to change this commonly is because if the input
argument has to be added
per test, that will prevent the existing tests to be used in the
aforementioned scenarios. Please let
me how you think.
Thanks in advance.
Regards,
Kun
On 5/31/2023 1:39 PM, Kinney, Michael D wrote:
> Would it be more flexible for unit tests apps to support an optional command line arg
> to specify the volume to write results?
>
> Mike
>
>> -----Original Message-----
>> From: Kun Qin <kuqin12@gmail.com>
>> Sent: Wednesday, May 31, 2023 12:14 PM
>> To: devel@edk2.groups.io
>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
>> <mikuback@linux.microsoft.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>
>> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save
>> Unit Test Cache to Drive
>>
>> From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
>>
>> Current implementation of UnitTestFrameworkPkg for shell-based unit test
>> will save the unit test cache to the same volume as the test application
>> itself. This works as long as the test application is on a writable
>> volume, such as USB or EFI partition.
>>
>> Instead of saving the files to the same file system of unit test
>> application, this change will save the cache file to the path where the
>> user ran this test application.
>>
>> This change was tested on proprietary physical hardware platforms and
>> QEMU based virtual platform.
>>
>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>> ---
>>
>> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes
>> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
>> 1 file changed, 43 insertions(+), 38 deletions(-)
>>
>> diff --git
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>> estPersistenceLibSimpleFileSystem.c
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>> estPersistenceLibSimpleFileSystem.c
>> index b59991683f48..d4181808b2be 100644
>> ---
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>> estPersistenceLibSimpleFileSystem.c
>> +++
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>> estPersistenceLibSimpleFileSystem.c
>> @@ -22,7 +22,7 @@
>> #define CACHE_FILE_SUFFIX L"_Cache.dat"
>>
>>
>>
>> /**
>>
>> - Generate the device path to the cache file.
>>
>> + Generate the file name and path to the cache file.
>>
>>
>>
>> @param[in] FrameworkHandle A pointer to the framework that is being
>> persisted.
>>
>>
>>
>> @@ -31,8 +31,8 @@
>>
>>
>> **/
>>
>> STATIC
>>
>> -EFI_DEVICE_PATH_PROTOCOL *
>>
>> -GetCacheFileDevicePath (
>>
>> +CHAR16 *
>>
>> +GetCacheFileName (
>>
>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>
>> )
>>
>> {
>>
>> @@ -85,7 +85,12 @@ GetCacheFileDevicePath (
>> // PathCleanUpDirectories (FileNameCopy);
>>
>> // if (PathRemoveLastItem (FileNameCopy)) {
>>
>> //
>>
>> - AppPath = ConvertDevicePathToText (LoadedImage->FilePath,
>> TRUE, TRUE); // NOTE: This must be freed.
>>
>> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE, TRUE);
>> // NOTE: This must be freed.
>>
>> + if (AppPath == NULL) {
>>
>> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to text.\n",
>> __func__));
>>
>> + goto Exit;
>>
>> + }
>>
>> +
>>
>> DirectorySlashOffset = StrLen (AppPath);
>>
>> //
>>
>> // Make sure we didn't get any weird data.
>>
>> @@ -148,15 +153,15 @@ Exit:
>> FreePool (AppPath);
>>
>> }
>>
>>
>>
>> - if (CacheFilePath != NULL) {
>>
>> - FreePool (CacheFilePath);
>>
>> + if (CacheFileDevicePath != NULL) {
>>
>> + FreePool (CacheFileDevicePath);
>>
>> }
>>
>>
>>
>> if (TestName != NULL) {
>>
>> FreePool (TestName);
>>
>> }
>>
>>
>>
>> - return CacheFileDevicePath;
>>
>> + return CacheFilePath;
>>
>> }
>>
>>
>>
>> /**
>>
>> @@ -175,21 +180,21 @@ DoesCacheExist (
>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>
>> )
>>
>> {
>>
>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>
>> - EFI_STATUS Status;
>>
>> - SHELL_FILE_HANDLE FileHandle;
>>
>> + CHAR16 *FileName;
>>
>> + EFI_STATUS Status;
>>
>> + SHELL_FILE_HANDLE FileHandle;
>>
>>
>>
>> //
>>
>> // NOTE: This devpath is allocated and must be freed.
>>
>> //
>>
>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>
>> + FileName = GetCacheFileName (FrameworkHandle);
>>
>>
>>
>> //
>>
>> // Check to see whether the file exists. If the file can be opened for
>>
>> // reading, it exists. Otherwise, probably not.
>>
>> //
>>
>> - Status = ShellOpenFileByDevicePath (
>>
>> - &FileDevicePath,
>>
>> + Status = ShellOpenFileByName (
>>
>> + FileName,
>>
>> &FileHandle,
>>
>> EFI_FILE_MODE_READ,
>>
>> 0
>>
>> @@ -198,8 +203,8 @@ DoesCacheExist (
>> ShellCloseFile (&FileHandle);
>>
>> }
>>
>>
>>
>> - if (FileDevicePath != NULL) {
>>
>> - FreePool (FileDevicePath);
>>
>> + if (FileName != NULL) {
>>
>> + FreePool (FileName);
>>
>> }
>>
>>
>>
>> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR
>> (Status)));
>>
>> @@ -229,10 +234,10 @@ SaveUnitTestCache (
>> IN UINTN SaveStateSize
>>
>> )
>>
>> {
>>
>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>
>> - EFI_STATUS Status;
>>
>> - SHELL_FILE_HANDLE FileHandle;
>>
>> - UINTN WriteCount;
>>
>> + CHAR16 *FileName;
>>
>> + EFI_STATUS Status;
>>
>> + SHELL_FILE_HANDLE FileHandle;
>>
>> + UINTN WriteCount;
>>
>>
>>
>> //
>>
>> // Check the inputs for sanity.
>>
>> @@ -245,13 +250,13 @@ SaveUnitTestCache (
>> // Determine the path for the cache file.
>>
>> // NOTE: This devpath is allocated and must be freed.
>>
>> //
>>
>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>
>> + FileName = GetCacheFileName (FrameworkHandle);
>>
>>
>>
>> //
>>
>> // First lets open the file if it exists so we can delete it...This is
>> the work around for truncation
>>
>> //
>>
>> - Status = ShellOpenFileByDevicePath (
>>
>> - &FileDevicePath,
>>
>> + Status = ShellOpenFileByName (
>>
>> + FileName,
>>
>> &FileHandle,
>>
>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
>>
>> 0
>>
>> @@ -270,8 +275,8 @@ SaveUnitTestCache (
>> //
>>
>> // Now that we know the path to the file... let's open it for writing.
>>
>> //
>>
>> - Status = ShellOpenFileByDevicePath (
>>
>> - &FileDevicePath,
>>
>> + Status = ShellOpenFileByName (
>>
>> + FileName,
>>
>> &FileHandle,
>>
>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
>> EFI_FILE_MODE_CREATE),
>>
>> 0
>>
>> @@ -304,8 +309,8 @@ SaveUnitTestCache (
>> ShellCloseFile (&FileHandle);
>>
>>
>>
>> Exit:
>>
>> - if (FileDevicePath != NULL) {
>>
>> - FreePool (FileDevicePath);
>>
>> + if (FileName != NULL) {
>>
>> + FreePool (FileName);
>>
>> }
>>
>>
>>
>> return Status;
>>
>> @@ -334,13 +339,13 @@ LoadUnitTestCache (
>> OUT UINTN *SaveStateSize
>>
>> )
>>
>> {
>>
>> - EFI_STATUS Status;
>>
>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>
>> - SHELL_FILE_HANDLE FileHandle;
>>
>> - BOOLEAN IsFileOpened;
>>
>> - UINT64 LargeFileSize;
>>
>> - UINTN FileSize;
>>
>> - VOID *Buffer;
>>
>> + EFI_STATUS Status;
>>
>> + CHAR16 *FileName;
>>
>> + SHELL_FILE_HANDLE FileHandle;
>>
>> + BOOLEAN IsFileOpened;
>>
>> + UINT64 LargeFileSize;
>>
>> + UINTN FileSize;
>>
>> + VOID *Buffer;
>>
>>
>>
>> IsFileOpened = FALSE;
>>
>> Buffer = NULL;
>>
>> @@ -356,13 +361,13 @@ LoadUnitTestCache (
>> // Determine the path for the cache file.
>>
>> // NOTE: This devpath is allocated and must be freed.
>>
>> //
>>
>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>
>> + FileName = GetCacheFileName (FrameworkHandle);
>>
>>
>>
>> //
>>
>> // Now that we know the path to the file... let's open it for writing.
>>
>> //
>>
>> - Status = ShellOpenFileByDevicePath (
>>
>> - &FileDevicePath,
>>
>> + Status = ShellOpenFileByName (
>>
>> + FileName,
>>
>> &FileHandle,
>>
>> EFI_FILE_MODE_READ,
>>
>> 0
>>
>> @@ -407,8 +412,8 @@ Exit:
>> //
>>
>> // Free allocated buffers
>>
>> //
>>
>> - if (FileDevicePath != NULL) {
>>
>> - FreePool (FileDevicePath);
>>
>> + if (FileName != NULL) {
>>
>> + FreePool (FileName);
>>
>> }
>>
>>
>>
>> if (IsFileOpened) {
>>
>> --
>> 2.40.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-06-01 15:50 ` Kun Qin
@ 2023-06-01 16:48 ` Michael D Kinney
2023-06-01 17:47 ` [edk2-devel] " Kun Qin
2023-06-07 1:14 ` Kun Qin
0 siblings, 2 replies; 7+ messages in thread
From: Michael D Kinney @ 2023-06-01 16:48 UTC (permalink / raw)
To: Kun Qin, devel@edk2.groups.io
Cc: Sean Brogan, Michael Kubacki, Kinney, Michael D
I think your use case is limited to the UnitTestLib based tests running from the UEFI Shell.
In that case, the library UnitTestFrameworkPkg\Library\UnitTestLib\UnitTestLib.inf is always
linked to the unit test application. You could add a CONSTRUCTOR to UnitTestLib.inf and the
CONSTRUCTOR function can look for UEFI Shell arguments and if args specify alternate path
for unit test cache then save some state for that path.
Mike
> -----Original Message-----
> From: Kun Qin <kuqin12@gmail.com>
> Sent: Thursday, June 1, 2023 8:50 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
> <mikuback@linux.microsoft.com>
> Subject: Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
> Save Unit Test Cache to Drive
>
> Hi Mike,
>
> Yes, that would indeed serve the purpose. Could you please show me a
> pointer on how to add that
> commonly? I would be open to change the code the other way.
>
> The reason I would like to change this commonly is because if the input
> argument has to be added
> per test, that will prevent the existing tests to be used in the
> aforementioned scenarios. Please let
> me how you think.
>
> Thanks in advance.
>
> Regards,
> Kun
>
> On 5/31/2023 1:39 PM, Kinney, Michael D wrote:
> > Would it be more flexible for unit tests apps to support an optional
> command line arg
> > to specify the volume to write results?
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: Kun Qin <kuqin12@gmail.com>
> >> Sent: Wednesday, May 31, 2023 12:14 PM
> >> To: devel@edk2.groups.io
> >> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
> >> <mikuback@linux.microsoft.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>
> >> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
> Save
> >> Unit Test Cache to Drive
> >>
> >> From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
> >>
> >> Current implementation of UnitTestFrameworkPkg for shell-based unit test
> >> will save the unit test cache to the same volume as the test application
> >> itself. This works as long as the test application is on a writable
> >> volume, such as USB or EFI partition.
> >>
> >> Instead of saving the files to the same file system of unit test
> >> application, this change will save the cache file to the path where the
> >> user ran this test application.
> >>
> >> This change was tested on proprietary physical hardware platforms and
> >> QEMU based virtual platform.
> >>
> >> Cc: Sean Brogan <sean.brogan@microsoft.com>
> >> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
> >> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> >> Signed-off-by: Kun Qin <kuqin12@gmail.com>
> >> ---
> >>
> >>
> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes
> >> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
> >> 1 file changed, 43 insertions(+), 38 deletions(-)
> >>
> >> diff --git
> >>
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> >> estPersistenceLibSimpleFileSystem.c
> >>
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> >> estPersistenceLibSimpleFileSystem.c
> >> index b59991683f48..d4181808b2be 100644
> >> ---
> >>
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> >> estPersistenceLibSimpleFileSystem.c
> >> +++
> >>
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
> >> estPersistenceLibSimpleFileSystem.c
> >> @@ -22,7 +22,7 @@
> >> #define CACHE_FILE_SUFFIX L"_Cache.dat"
> >>
> >>
> >>
> >> /**
> >>
> >> - Generate the device path to the cache file.
> >>
> >> + Generate the file name and path to the cache file.
> >>
> >>
> >>
> >> @param[in] FrameworkHandle A pointer to the framework that is
> being
> >> persisted.
> >>
> >>
> >>
> >> @@ -31,8 +31,8 @@
> >>
> >>
> >> **/
> >>
> >> STATIC
> >>
> >> -EFI_DEVICE_PATH_PROTOCOL *
> >>
> >> -GetCacheFileDevicePath (
> >>
> >> +CHAR16 *
> >>
> >> +GetCacheFileName (
> >>
> >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
> >>
> >> )
> >>
> >> {
> >>
> >> @@ -85,7 +85,12 @@ GetCacheFileDevicePath (
> >> // PathCleanUpDirectories (FileNameCopy);
> >>
> >> // if (PathRemoveLastItem (FileNameCopy)) {
> >>
> >> //
> >>
> >> - AppPath = ConvertDevicePathToText (LoadedImage-
> >FilePath,
> >> TRUE, TRUE); // NOTE: This must be freed.
> >>
> >> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE,
> TRUE);
> >> // NOTE: This must be freed.
> >>
> >> + if (AppPath == NULL) {
> >>
> >> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to
> text.\n",
> >> __func__));
> >>
> >> + goto Exit;
> >>
> >> + }
> >>
> >> +
> >>
> >> DirectorySlashOffset = StrLen (AppPath);
> >>
> >> //
> >>
> >> // Make sure we didn't get any weird data.
> >>
> >> @@ -148,15 +153,15 @@ Exit:
> >> FreePool (AppPath);
> >>
> >> }
> >>
> >>
> >>
> >> - if (CacheFilePath != NULL) {
> >>
> >> - FreePool (CacheFilePath);
> >>
> >> + if (CacheFileDevicePath != NULL) {
> >>
> >> + FreePool (CacheFileDevicePath);
> >>
> >> }
> >>
> >>
> >>
> >> if (TestName != NULL) {
> >>
> >> FreePool (TestName);
> >>
> >> }
> >>
> >>
> >>
> >> - return CacheFileDevicePath;
> >>
> >> + return CacheFilePath;
> >>
> >> }
> >>
> >>
> >>
> >> /**
> >>
> >> @@ -175,21 +180,21 @@ DoesCacheExist (
> >> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
> >>
> >> )
> >>
> >> {
> >>
> >> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
> >>
> >> - EFI_STATUS Status;
> >>
> >> - SHELL_FILE_HANDLE FileHandle;
> >>
> >> + CHAR16 *FileName;
> >>
> >> + EFI_STATUS Status;
> >>
> >> + SHELL_FILE_HANDLE FileHandle;
> >>
> >>
> >>
> >> //
> >>
> >> // NOTE: This devpath is allocated and must be freed.
> >>
> >> //
> >>
> >> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
> >>
> >> + FileName = GetCacheFileName (FrameworkHandle);
> >>
> >>
> >>
> >> //
> >>
> >> // Check to see whether the file exists. If the file can be opened
> for
> >>
> >> // reading, it exists. Otherwise, probably not.
> >>
> >> //
> >>
> >> - Status = ShellOpenFileByDevicePath (
> >>
> >> - &FileDevicePath,
> >>
> >> + Status = ShellOpenFileByName (
> >>
> >> + FileName,
> >>
> >> &FileHandle,
> >>
> >> EFI_FILE_MODE_READ,
> >>
> >> 0
> >>
> >> @@ -198,8 +203,8 @@ DoesCacheExist (
> >> ShellCloseFile (&FileHandle);
> >>
> >> }
> >>
> >>
> >>
> >> - if (FileDevicePath != NULL) {
> >>
> >> - FreePool (FileDevicePath);
> >>
> >> + if (FileName != NULL) {
> >>
> >> + FreePool (FileName);
> >>
> >> }
> >>
> >>
> >>
> >> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR
> >> (Status)));
> >>
> >> @@ -229,10 +234,10 @@ SaveUnitTestCache (
> >> IN UINTN SaveStateSize
> >>
> >> )
> >>
> >> {
> >>
> >> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
> >>
> >> - EFI_STATUS Status;
> >>
> >> - SHELL_FILE_HANDLE FileHandle;
> >>
> >> - UINTN WriteCount;
> >>
> >> + CHAR16 *FileName;
> >>
> >> + EFI_STATUS Status;
> >>
> >> + SHELL_FILE_HANDLE FileHandle;
> >>
> >> + UINTN WriteCount;
> >>
> >>
> >>
> >> //
> >>
> >> // Check the inputs for sanity.
> >>
> >> @@ -245,13 +250,13 @@ SaveUnitTestCache (
> >> // Determine the path for the cache file.
> >>
> >> // NOTE: This devpath is allocated and must be freed.
> >>
> >> //
> >>
> >> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
> >>
> >> + FileName = GetCacheFileName (FrameworkHandle);
> >>
> >>
> >>
> >> //
> >>
> >> // First lets open the file if it exists so we can delete it...This
> is
> >> the work around for truncation
> >>
> >> //
> >>
> >> - Status = ShellOpenFileByDevicePath (
> >>
> >> - &FileDevicePath,
> >>
> >> + Status = ShellOpenFileByName (
> >>
> >> + FileName,
> >>
> >> &FileHandle,
> >>
> >> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
> >>
> >> 0
> >>
> >> @@ -270,8 +275,8 @@ SaveUnitTestCache (
> >> //
> >>
> >> // Now that we know the path to the file... let's open it for
> writing.
> >>
> >> //
> >>
> >> - Status = ShellOpenFileByDevicePath (
> >>
> >> - &FileDevicePath,
> >>
> >> + Status = ShellOpenFileByName (
> >>
> >> + FileName,
> >>
> >> &FileHandle,
> >>
> >> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
> >> EFI_FILE_MODE_CREATE),
> >>
> >> 0
> >>
> >> @@ -304,8 +309,8 @@ SaveUnitTestCache (
> >> ShellCloseFile (&FileHandle);
> >>
> >>
> >>
> >> Exit:
> >>
> >> - if (FileDevicePath != NULL) {
> >>
> >> - FreePool (FileDevicePath);
> >>
> >> + if (FileName != NULL) {
> >>
> >> + FreePool (FileName);
> >>
> >> }
> >>
> >>
> >>
> >> return Status;
> >>
> >> @@ -334,13 +339,13 @@ LoadUnitTestCache (
> >> OUT UINTN *SaveStateSize
> >>
> >> )
> >>
> >> {
> >>
> >> - EFI_STATUS Status;
> >>
> >> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
> >>
> >> - SHELL_FILE_HANDLE FileHandle;
> >>
> >> - BOOLEAN IsFileOpened;
> >>
> >> - UINT64 LargeFileSize;
> >>
> >> - UINTN FileSize;
> >>
> >> - VOID *Buffer;
> >>
> >> + EFI_STATUS Status;
> >>
> >> + CHAR16 *FileName;
> >>
> >> + SHELL_FILE_HANDLE FileHandle;
> >>
> >> + BOOLEAN IsFileOpened;
> >>
> >> + UINT64 LargeFileSize;
> >>
> >> + UINTN FileSize;
> >>
> >> + VOID *Buffer;
> >>
> >>
> >>
> >> IsFileOpened = FALSE;
> >>
> >> Buffer = NULL;
> >>
> >> @@ -356,13 +361,13 @@ LoadUnitTestCache (
> >> // Determine the path for the cache file.
> >>
> >> // NOTE: This devpath is allocated and must be freed.
> >>
> >> //
> >>
> >> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
> >>
> >> + FileName = GetCacheFileName (FrameworkHandle);
> >>
> >>
> >>
> >> //
> >>
> >> // Now that we know the path to the file... let's open it for
> writing.
> >>
> >> //
> >>
> >> - Status = ShellOpenFileByDevicePath (
> >>
> >> - &FileDevicePath,
> >>
> >> + Status = ShellOpenFileByName (
> >>
> >> + FileName,
> >>
> >> &FileHandle,
> >>
> >> EFI_FILE_MODE_READ,
> >>
> >> 0
> >>
> >> @@ -407,8 +412,8 @@ Exit:
> >> //
> >>
> >> // Free allocated buffers
> >>
> >> //
> >>
> >> - if (FileDevicePath != NULL) {
> >>
> >> - FreePool (FileDevicePath);
> >>
> >> + if (FileName != NULL) {
> >>
> >> + FreePool (FileName);
> >>
> >> }
> >>
> >>
> >>
> >> if (IsFileOpened) {
> >>
> >> --
> >> 2.40.1.windows.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-06-01 16:48 ` Michael D Kinney
@ 2023-06-01 17:47 ` Kun Qin
2023-06-07 1:14 ` Kun Qin
1 sibling, 0 replies; 7+ messages in thread
From: Kun Qin @ 2023-06-01 17:47 UTC (permalink / raw)
To: devel, michael.d.kinney; +Cc: Sean Brogan, Michael Kubacki
Thanks for the suggestion, Mike. I will try and update the patch per
your suggestion.
Regards,
Kun
On 6/1/2023 9:48 AM, Michael D Kinney wrote:
> I think your use case is limited to the UnitTestLib based tests running from the UEFI Shell.
>
> In that case, the library UnitTestFrameworkPkg\Library\UnitTestLib\UnitTestLib.inf is always
> linked to the unit test application. You could add a CONSTRUCTOR to UnitTestLib.inf and the
> CONSTRUCTOR function can look for UEFI Shell arguments and if args specify alternate path
> for unit test cache then save some state for that path.
>
> Mike
>
>
>> -----Original Message-----
>> From: Kun Qin <kuqin12@gmail.com>
>> Sent: Thursday, June 1, 2023 8:50 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
>> <mikuback@linux.microsoft.com>
>> Subject: Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
>> Save Unit Test Cache to Drive
>>
>> Hi Mike,
>>
>> Yes, that would indeed serve the purpose. Could you please show me a
>> pointer on how to add that
>> commonly? I would be open to change the code the other way.
>>
>> The reason I would like to change this commonly is because if the input
>> argument has to be added
>> per test, that will prevent the existing tests to be used in the
>> aforementioned scenarios. Please let
>> me how you think.
>>
>> Thanks in advance.
>>
>> Regards,
>> Kun
>>
>> On 5/31/2023 1:39 PM, Kinney, Michael D wrote:
>>> Would it be more flexible for unit tests apps to support an optional
>> command line arg
>>> to specify the volume to write results?
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Kun Qin <kuqin12@gmail.com>
>>>> Sent: Wednesday, May 31, 2023 12:14 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
>>>> <mikuback@linux.microsoft.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
>> Save
>>>> Unit Test Cache to Drive
>>>>
>>>> From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
>>>>
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
>>>>
>>>> Current implementation of UnitTestFrameworkPkg for shell-based unit test
>>>> will save the unit test cache to the same volume as the test application
>>>> itself. This works as long as the test application is on a writable
>>>> volume, such as USB or EFI partition.
>>>>
>>>> Instead of saving the files to the same file system of unit test
>>>> application, this change will save the cache file to the path where the
>>>> user ran this test application.
>>>>
>>>> This change was tested on proprietary physical hardware platforms and
>>>> QEMU based virtual platform.
>>>>
>>>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>>>> ---
>>>>
>>>>
>> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes
>>>> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
>>>> 1 file changed, 43 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git
>>>>
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>>
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> index b59991683f48..d4181808b2be 100644
>>>> ---
>>>>
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> +++
>>>>
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> @@ -22,7 +22,7 @@
>>>> #define CACHE_FILE_SUFFIX L"_Cache.dat"
>>>>
>>>>
>>>>
>>>> /**
>>>>
>>>> - Generate the device path to the cache file.
>>>>
>>>> + Generate the file name and path to the cache file.
>>>>
>>>>
>>>>
>>>> @param[in] FrameworkHandle A pointer to the framework that is
>> being
>>>> persisted.
>>>>
>>>>
>>>>
>>>> @@ -31,8 +31,8 @@
>>>>
>>>>
>>>> **/
>>>>
>>>> STATIC
>>>>
>>>> -EFI_DEVICE_PATH_PROTOCOL *
>>>>
>>>> -GetCacheFileDevicePath (
>>>>
>>>> +CHAR16 *
>>>>
>>>> +GetCacheFileName (
>>>>
>>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> @@ -85,7 +85,12 @@ GetCacheFileDevicePath (
>>>> // PathCleanUpDirectories (FileNameCopy);
>>>>
>>>> // if (PathRemoveLastItem (FileNameCopy)) {
>>>>
>>>> //
>>>>
>>>> - AppPath = ConvertDevicePathToText (LoadedImage-
>>> FilePath,
>>>> TRUE, TRUE); // NOTE: This must be freed.
>>>>
>>>> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE,
>> TRUE);
>>>> // NOTE: This must be freed.
>>>>
>>>> + if (AppPath == NULL) {
>>>>
>>>> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to
>> text.\n",
>>>> __func__));
>>>>
>>>> + goto Exit;
>>>>
>>>> + }
>>>>
>>>> +
>>>>
>>>> DirectorySlashOffset = StrLen (AppPath);
>>>>
>>>> //
>>>>
>>>> // Make sure we didn't get any weird data.
>>>>
>>>> @@ -148,15 +153,15 @@ Exit:
>>>> FreePool (AppPath);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - if (CacheFilePath != NULL) {
>>>>
>>>> - FreePool (CacheFilePath);
>>>>
>>>> + if (CacheFileDevicePath != NULL) {
>>>>
>>>> + FreePool (CacheFileDevicePath);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> if (TestName != NULL) {
>>>>
>>>> FreePool (TestName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - return CacheFileDevicePath;
>>>>
>>>> + return CacheFilePath;
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> /**
>>>>
>>>> @@ -175,21 +180,21 @@ DoesCacheExist (
>>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Check to see whether the file exists. If the file can be opened
>> for
>>>> // reading, it exists. Otherwise, probably not.
>>>>
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> EFI_FILE_MODE_READ,
>>>>
>>>> 0
>>>>
>>>> @@ -198,8 +203,8 @@ DoesCacheExist (
>>>> ShellCloseFile (&FileHandle);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR
>>>> (Status)));
>>>>
>>>> @@ -229,10 +234,10 @@ SaveUnitTestCache (
>>>> IN UINTN SaveStateSize
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> - UINTN WriteCount;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + UINTN WriteCount;
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Check the inputs for sanity.
>>>>
>>>> @@ -245,13 +250,13 @@ SaveUnitTestCache (
>>>> // Determine the path for the cache file.
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // First lets open the file if it exists so we can delete it...This
>> is
>>>> the work around for truncation
>>>>
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
>>>>
>>>> 0
>>>>
>>>> @@ -270,8 +275,8 @@ SaveUnitTestCache (
>>>> //
>>>>
>>>> // Now that we know the path to the file... let's open it for
>> writing.
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
>>>> EFI_FILE_MODE_CREATE),
>>>>
>>>> 0
>>>>
>>>> @@ -304,8 +309,8 @@ SaveUnitTestCache (
>>>> ShellCloseFile (&FileHandle);
>>>>
>>>>
>>>>
>>>> Exit:
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> return Status;
>>>>
>>>> @@ -334,13 +339,13 @@ LoadUnitTestCache (
>>>> OUT UINTN *SaveStateSize
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> - BOOLEAN IsFileOpened;
>>>>
>>>> - UINT64 LargeFileSize;
>>>>
>>>> - UINTN FileSize;
>>>>
>>>> - VOID *Buffer;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + BOOLEAN IsFileOpened;
>>>>
>>>> + UINT64 LargeFileSize;
>>>>
>>>> + UINTN FileSize;
>>>>
>>>> + VOID *Buffer;
>>>>
>>>>
>>>>
>>>> IsFileOpened = FALSE;
>>>>
>>>> Buffer = NULL;
>>>>
>>>> @@ -356,13 +361,13 @@ LoadUnitTestCache (
>>>> // Determine the path for the cache file.
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Now that we know the path to the file... let's open it for
>> writing.
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> EFI_FILE_MODE_READ,
>>>>
>>>> 0
>>>>
>>>> @@ -407,8 +412,8 @@ Exit:
>>>> //
>>>>
>>>> // Free allocated buffers
>>>>
>>>> //
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> if (IsFileOpened) {
>>>>
>>>> --
>>>> 2.40.1.windows.1
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive
2023-06-01 16:48 ` Michael D Kinney
2023-06-01 17:47 ` [edk2-devel] " Kun Qin
@ 2023-06-07 1:14 ` Kun Qin
1 sibling, 0 replies; 7+ messages in thread
From: Kun Qin @ 2023-06-07 1:14 UTC (permalink / raw)
To: devel, michael.d.kinney; +Cc: Sean Brogan, Michael Kubacki
Hi Mike,
I reworked the patch and sent a v2 here:
https://edk2.groups.io/g/devel/message/105827
Note that I did not end up putting the shell based argument
checks in UnitTestLib.inf because that library has multi-phase
support. Adding Shell argument checks will break it unless
we add more library instances to maintain the compatibility.
I think adding argument support here would be more self-contained
and we do not need to pass the path around through changing
framework handle structure.
Please let me know if you have any feedback.
Thanks,
Kun
On 6/1/2023 9:48 AM, Michael D Kinney wrote:
> I think your use case is limited to the UnitTestLib based tests running from the UEFI Shell.
>
> In that case, the library UnitTestFrameworkPkg\Library\UnitTestLib\UnitTestLib.inf is always
> linked to the unit test application. You could add a CONSTRUCTOR to UnitTestLib.inf and the
> CONSTRUCTOR function can look for UEFI Shell arguments and if args specify alternate path
> for unit test cache then save some state for that path.
>
> Mike
>
>
>> -----Original Message-----
>> From: Kun Qin <kuqin12@gmail.com>
>> Sent: Thursday, June 1, 2023 8:50 AM
>> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
>> <mikuback@linux.microsoft.com>
>> Subject: Re: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
>> Save Unit Test Cache to Drive
>>
>> Hi Mike,
>>
>> Yes, that would indeed serve the purpose. Could you please show me a
>> pointer on how to add that
>> commonly? I would be open to change the code the other way.
>>
>> The reason I would like to change this commonly is because if the input
>> argument has to be added
>> per test, that will prevent the existing tests to be used in the
>> aforementioned scenarios. Please let
>> me how you think.
>>
>> Thanks in advance.
>>
>> Regards,
>> Kun
>>
>> On 5/31/2023 1:39 PM, Kinney, Michael D wrote:
>>> Would it be more flexible for unit tests apps to support an optional
>> command line arg
>>> to specify the volume to write results?
>>>
>>> Mike
>>>
>>>> -----Original Message-----
>>>> From: Kun Qin <kuqin12@gmail.com>
>>>> Sent: Wednesday, May 31, 2023 12:14 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Sean Brogan <sean.brogan@microsoft.com>; Michael Kubacki
>>>> <mikuback@linux.microsoft.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>> Subject: [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib:
>> Save
>>>> Unit Test Cache to Drive
>>>>
>>>> From: kuqin12 <42554914+kuqin12@users.noreply.github.com>
>>>>
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4467
>>>>
>>>> Current implementation of UnitTestFrameworkPkg for shell-based unit test
>>>> will save the unit test cache to the same volume as the test application
>>>> itself. This works as long as the test application is on a writable
>>>> volume, such as USB or EFI partition.
>>>>
>>>> Instead of saving the files to the same file system of unit test
>>>> application, this change will save the cache file to the path where the
>>>> user ran this test application.
>>>>
>>>> This change was tested on proprietary physical hardware platforms and
>>>> QEMU based virtual platform.
>>>>
>>>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>>>> Cc: Michael Kubacki <mikuback@linux.microsoft.com>
>>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>>>> ---
>>>>
>>>>
>> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTes
>>>> tPersistenceLibSimpleFileSystem.c | 81 +++++++++++---------
>>>> 1 file changed, 43 insertions(+), 38 deletions(-)
>>>>
>>>> diff --git
>>>>
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>>
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> index b59991683f48..d4181808b2be 100644
>>>> ---
>>>>
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> +++
>>>>
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitT
>>>> estPersistenceLibSimpleFileSystem.c
>>>> @@ -22,7 +22,7 @@
>>>> #define CACHE_FILE_SUFFIX L"_Cache.dat"
>>>>
>>>>
>>>>
>>>> /**
>>>>
>>>> - Generate the device path to the cache file.
>>>>
>>>> + Generate the file name and path to the cache file.
>>>>
>>>>
>>>>
>>>> @param[in] FrameworkHandle A pointer to the framework that is
>> being
>>>> persisted.
>>>>
>>>>
>>>>
>>>> @@ -31,8 +31,8 @@
>>>>
>>>>
>>>> **/
>>>>
>>>> STATIC
>>>>
>>>> -EFI_DEVICE_PATH_PROTOCOL *
>>>>
>>>> -GetCacheFileDevicePath (
>>>>
>>>> +CHAR16 *
>>>>
>>>> +GetCacheFileName (
>>>>
>>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> @@ -85,7 +85,12 @@ GetCacheFileDevicePath (
>>>> // PathCleanUpDirectories (FileNameCopy);
>>>>
>>>> // if (PathRemoveLastItem (FileNameCopy)) {
>>>>
>>>> //
>>>>
>>>> - AppPath = ConvertDevicePathToText (LoadedImage-
>>> FilePath,
>>>> TRUE, TRUE); // NOTE: This must be freed.
>>>>
>>>> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE,
>> TRUE);
>>>> // NOTE: This must be freed.
>>>>
>>>> + if (AppPath == NULL) {
>>>>
>>>> + DEBUG ((DEBUG_ERROR, "%a - Failed to convert device path to
>> text.\n",
>>>> __func__));
>>>>
>>>> + goto Exit;
>>>>
>>>> + }
>>>>
>>>> +
>>>>
>>>> DirectorySlashOffset = StrLen (AppPath);
>>>>
>>>> //
>>>>
>>>> // Make sure we didn't get any weird data.
>>>>
>>>> @@ -148,15 +153,15 @@ Exit:
>>>> FreePool (AppPath);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - if (CacheFilePath != NULL) {
>>>>
>>>> - FreePool (CacheFilePath);
>>>>
>>>> + if (CacheFileDevicePath != NULL) {
>>>>
>>>> + FreePool (CacheFileDevicePath);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> if (TestName != NULL) {
>>>>
>>>> FreePool (TestName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - return CacheFileDevicePath;
>>>>
>>>> + return CacheFilePath;
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> /**
>>>>
>>>> @@ -175,21 +180,21 @@ DoesCacheExist (
>>>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Check to see whether the file exists. If the file can be opened
>> for
>>>> // reading, it exists. Otherwise, probably not.
>>>>
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> EFI_FILE_MODE_READ,
>>>>
>>>> 0
>>>>
>>>> @@ -198,8 +203,8 @@ DoesCacheExist (
>>>> ShellCloseFile (&FileHandle);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> DEBUG ((DEBUG_VERBOSE, "%a - Returning %d\n", __func__, !EFI_ERROR
>>>> (Status)));
>>>>
>>>> @@ -229,10 +234,10 @@ SaveUnitTestCache (
>>>> IN UINTN SaveStateSize
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> - UINTN WriteCount;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + UINTN WriteCount;
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Check the inputs for sanity.
>>>>
>>>> @@ -245,13 +250,13 @@ SaveUnitTestCache (
>>>> // Determine the path for the cache file.
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // First lets open the file if it exists so we can delete it...This
>> is
>>>> the work around for truncation
>>>>
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE),
>>>>
>>>> 0
>>>>
>>>> @@ -270,8 +275,8 @@ SaveUnitTestCache (
>>>> //
>>>>
>>>> // Now that we know the path to the file... let's open it for
>> writing.
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> (EFI_FILE_MODE_READ | EFI_FILE_MODE_WRITE |
>>>> EFI_FILE_MODE_CREATE),
>>>>
>>>> 0
>>>>
>>>> @@ -304,8 +309,8 @@ SaveUnitTestCache (
>>>> ShellCloseFile (&FileHandle);
>>>>
>>>>
>>>>
>>>> Exit:
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> return Status;
>>>>
>>>> @@ -334,13 +339,13 @@ LoadUnitTestCache (
>>>> OUT UINTN *SaveStateSize
>>>>
>>>> )
>>>>
>>>> {
>>>>
>>>> - EFI_STATUS Status;
>>>>
>>>> - EFI_DEVICE_PATH_PROTOCOL *FileDevicePath;
>>>>
>>>> - SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> - BOOLEAN IsFileOpened;
>>>>
>>>> - UINT64 LargeFileSize;
>>>>
>>>> - UINTN FileSize;
>>>>
>>>> - VOID *Buffer;
>>>>
>>>> + EFI_STATUS Status;
>>>>
>>>> + CHAR16 *FileName;
>>>>
>>>> + SHELL_FILE_HANDLE FileHandle;
>>>>
>>>> + BOOLEAN IsFileOpened;
>>>>
>>>> + UINT64 LargeFileSize;
>>>>
>>>> + UINTN FileSize;
>>>>
>>>> + VOID *Buffer;
>>>>
>>>>
>>>>
>>>> IsFileOpened = FALSE;
>>>>
>>>> Buffer = NULL;
>>>>
>>>> @@ -356,13 +361,13 @@ LoadUnitTestCache (
>>>> // Determine the path for the cache file.
>>>>
>>>> // NOTE: This devpath is allocated and must be freed.
>>>>
>>>> //
>>>>
>>>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>>>
>>>> + FileName = GetCacheFileName (FrameworkHandle);
>>>>
>>>>
>>>>
>>>> //
>>>>
>>>> // Now that we know the path to the file... let's open it for
>> writing.
>>>> //
>>>>
>>>> - Status = ShellOpenFileByDevicePath (
>>>>
>>>> - &FileDevicePath,
>>>>
>>>> + Status = ShellOpenFileByName (
>>>>
>>>> + FileName,
>>>>
>>>> &FileHandle,
>>>>
>>>> EFI_FILE_MODE_READ,
>>>>
>>>> 0
>>>>
>>>> @@ -407,8 +412,8 @@ Exit:
>>>> //
>>>>
>>>> // Free allocated buffers
>>>>
>>>> //
>>>>
>>>> - if (FileDevicePath != NULL) {
>>>>
>>>> - FreePool (FileDevicePath);
>>>>
>>>> + if (FileName != NULL) {
>>>>
>>>> + FreePool (FileName);
>>>>
>>>> }
>>>>
>>>>
>>>>
>>>> if (IsFileOpened) {
>>>>
>>>> --
>>>> 2.40.1.windows.1
>
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-06-07 1:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-31 19:13 [PATCH v1 0/1] Add support for running shell test application in an immutable volume Kun Qin
2023-05-31 19:13 ` [PATCH v1 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache to Drive Kun Qin
2023-05-31 20:39 ` Michael D Kinney
2023-06-01 15:50 ` Kun Qin
2023-06-01 16:48 ` Michael D Kinney
2023-06-01 17:47 ` [edk2-devel] " Kun Qin
2023-06-07 1:14 ` Kun Qin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox