* Re: [edk2-devel] [PATCH v3 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache Option
2023-06-30 23:00 ` [edk2-devel] " Michael Kubacki
@ 2023-07-07 15:45 ` Kun Qin
2023-07-20 0:48 ` Michael D Kinney
1 sibling, 0 replies; 5+ messages in thread
From: Kun Qin @ 2023-07-07 15:45 UTC (permalink / raw)
To: Michael Kubacki, devel, Michael D Kinney; +Cc: Sean Brogan
Hi Mike,
It has been a week since the last movement on this. Could you please
provide feedback on this patch when you have a chance? Any input is
appreciated.
Regards,
Kun
On 6/30/2023 4:00 PM, Michael Kubacki wrote:
> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> I haven't seen Mike Kinney acknowledge the changes made in response to
> his v1 request so we'll need to get that before merging the patch.
>
> Thanks,
> Michael
>
> On 6/30/2023 2:14 PM, Kun Qin wrote:
>> 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 then added an input argument to allow user to specify where
>> to save such cache file through `--CachePath` shell argument to allow
>> even more flexibility.
>>
>> 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>
>> ---
>>
>> Notes:
>> v2:
>> - Adding input argument [Mike Kinney]
>> v3:
>> - No review, no change
>>
>> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
>> | 232 +++++++++++++-------
>> 1 file changed, 157 insertions(+), 75 deletions(-)
>>
>> diff --git
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
>>
>> index b59991683f48..8f7f03646cd5 100644
>> ---
>> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
>> +++
>> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/UnitTestPersistenceLibSimpleFileSystem.c
>> @@ -16,13 +16,16 @@
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> #include <Library/DevicePathLib.h>
>>
>> #include <Library/ShellLib.h>
>>
>> +#include <Library/UefiLib.h>
>>
>> #include <Protocol/LoadedImage.h>
>>
>> #include <UnitTestFrameworkTypes.h>
>>
>>
>> #define CACHE_FILE_SUFFIX L"_Cache.dat"
>>
>>
>> +CHAR16 *mCachePath = NULL;
>>
>> +
>>
>> /**
>>
>> - 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 +34,8 @@
>>
>> **/
>>
>> STATIC
>>
>> -EFI_DEVICE_PATH_PROTOCOL *
>>
>> -GetCacheFileDevicePath (
>>
>> +CHAR16 *
>>
>> +GetCacheFileName (
>>
>> IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
>>
>> )
>>
>> {
>>
>> @@ -44,13 +47,11 @@ GetCacheFileDevicePath (
>> CHAR16 *TestName;
>>
>> UINTN DirectorySlashOffset;
>>
>> UINTN CacheFilePathLength;
>>
>> - EFI_DEVICE_PATH_PROTOCOL *CacheFileDevicePath;
>>
>>
>> - Framework = (UNIT_TEST_FRAMEWORK *)FrameworkHandle;
>>
>> - AppPath = NULL;
>>
>> - CacheFilePath = NULL;
>>
>> - TestName = NULL;
>>
>> - CacheFileDevicePath = NULL;
>>
>> + Framework = (UNIT_TEST_FRAMEWORK *)FrameworkHandle;
>>
>> + AppPath = NULL;
>>
>> + CacheFilePath = NULL;
>>
>> + TestName = NULL;
>>
>>
>> //
>>
>> // First, we need to get some information from the loaded image.
>>
>> @@ -85,35 +86,62 @@ GetCacheFileDevicePath (
>> // PathCleanUpDirectories (FileNameCopy);
>>
>> // if (PathRemoveLastItem (FileNameCopy)) {
>>
>> //
>>
>> - AppPath = ConvertDevicePathToText
>> (LoadedImage->FilePath, TRUE, TRUE); // NOTE: This must be freed.
>>
>> - DirectorySlashOffset = StrLen (AppPath);
>>
>> - //
>>
>> - // Make sure we didn't get any weird data.
>>
>> - //
>>
>> - if (DirectorySlashOffset == 0) {
>>
>> - DEBUG ((DEBUG_ERROR, "%a - Weird 0-length string when processing
>> app path.\n", __func__));
>>
>> - goto Exit;
>>
>> - }
>>
>> + if (mCachePath == NULL) {
>>
>> + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE,
>> TRUE); // NOTE: This must be freed.
>>
>> + if (AppPath == NULL) {
>>
>> + goto Exit;
>>
>> + }
>>
>>
>> - //
>>
>> - // Now that we know we have a decent string, let's take a deeper
>> look.
>>
>> - //
>>
>> - do {
>>
>> - if (AppPath[DirectorySlashOffset] == L'\\') {
>>
>> - break;
>>
>> + DirectorySlashOffset = StrLen (AppPath);
>>
>> + //
>>
>> + // Make sure we didn't get any weird data.
>>
>> + //
>>
>> + if (DirectorySlashOffset == 0) {
>>
>> + DEBUG ((DEBUG_ERROR, "%a - Weird 0-length string when
>> processing app path.\n", __func__));
>>
>> + goto Exit;
>>
>> }
>>
>>
>> - DirectorySlashOffset--;
>>
>> - } while (DirectorySlashOffset > 0);
>>
>> + //
>>
>> + // Now that we know we have a decent string, let's take a deeper
>> look.
>>
>> + //
>>
>> + do {
>>
>> + if (AppPath[DirectorySlashOffset] == L'\\') {
>>
>> + break;
>>
>> + }
>>
>> +
>>
>> + DirectorySlashOffset--;
>>
>> + } while (DirectorySlashOffset > 0);
>>
>>
>> - //
>>
>> - // After that little maneuver, DirectorySlashOffset should be
>> pointing at the last '\' in AppString.
>>
>> - // That would be the path to the parent directory that the test
>> app is executing from.
>>
>> - // Let's check and make sure that's right.
>>
>> - //
>>
>> - if (AppPath[DirectorySlashOffset] != L'\\') {
>>
>> - DEBUG ((DEBUG_ERROR, "%a - Could not find a single directory
>> separator in app path.\n", __func__));
>>
>> - goto Exit;
>>
>> + //
>>
>> + // After that little maneuver, DirectorySlashOffset should be
>> pointing at the last '\' in AppString.
>>
>> + // That would be the path to the parent directory that the test
>> app is executing from.
>>
>> + // Let's check and make sure that's right.
>>
>> + //
>>
>> + if (AppPath[DirectorySlashOffset] != L'\\') {
>>
>> + DEBUG ((DEBUG_ERROR, "%a - Could not find a single directory
>> separator in app path.\n", __func__));
>>
>> + goto Exit;
>>
>> + }
>>
>> + } else {
>>
>> + AppPath = FullyQualifyPath (mCachePath); // NOTE: This must be
>> freed.
>>
>> + if (AppPath == NULL) {
>>
>> + goto Exit;
>>
>> + }
>>
>> +
>>
>> + DirectorySlashOffset = StrLen (AppPath);
>>
>> +
>>
>> + if (AppPath[DirectorySlashOffset - 1] != L'\\') {
>>
>> + // Set the slash if user did not specify it on the newly
>> allocated pool
>>
>> + AppPath = ReallocatePool (
>>
>> + (DirectorySlashOffset + 1) * sizeof (CHAR16),
>>
>> + (DirectorySlashOffset + 2) * sizeof (CHAR16),
>>
>> + AppPath
>>
>> + );
>>
>> + AppPath[DirectorySlashOffset] = L'\\';
>>
>> + AppPath[DirectorySlashOffset + 1] = L'\0';
>>
>> + } else {
>>
>> + // Otherwise the user input is good enough to go, mostly
>>
>> + DirectorySlashOffset--;
>>
>> + }
>>
>> }
>>
>>
>> //
>>
>> @@ -135,11 +163,6 @@ GetCacheFileDevicePath (
>> StrCatS (CacheFilePath, CacheFilePathLength,
>> TestName); // Copy the base name for the
>> test cache.
>>
>> StrCatS (CacheFilePath, CacheFilePathLength,
>> CACHE_FILE_SUFFIX); // Copy the file suffix.
>>
>>
>> - //
>>
>> - // Finally, try to create the device path for the thing thing.
>>
>> - //
>>
>> - CacheFileDevicePath = FileDevicePath (LoadedImage->DeviceHandle,
>> CacheFilePath);
>>
>> -
>>
>> Exit:
>>
>> //
>>
>> // Free allocated buffers.
>>
>> @@ -148,15 +171,11 @@ Exit:
>> FreePool (AppPath);
>>
>> }
>>
>>
>> - if (CacheFilePath != NULL) {
>>
>> - FreePool (CacheFilePath);
>>
>> - }
>>
>> -
>>
>> if (TestName != NULL) {
>>
>> FreePool (TestName);
>>
>> }
>>
>>
>> - return CacheFileDevicePath;
>>
>> + return CacheFilePath;
>>
>> }
>>
>>
>> /**
>>
>> @@ -175,21 +194,24 @@ 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);
>>
>> + if (FileName == NULL) {
>>
>> + return FALSE;
>>
>> + }
>>
>>
>> //
>>
>> // 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 +220,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 +251,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 +267,16 @@ SaveUnitTestCache (
>> // Determine the path for the cache file.
>>
>> // NOTE: This devpath is allocated and must be freed.
>>
>> //
>>
>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>
>> + FileName = GetCacheFileName (FrameworkHandle);
>>
>> + if (FileName == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>>
>> //
>>
>> // 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 +295,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 +329,8 @@ SaveUnitTestCache (
>> ShellCloseFile (&FileHandle);
>>
>>
>> Exit:
>>
>> - if (FileDevicePath != NULL) {
>>
>> - FreePool (FileDevicePath);
>>
>> + if (FileName != NULL) {
>>
>> + FreePool (FileName);
>>
>> }
>>
>>
>> return Status;
>>
>> @@ -334,13 +359,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 +381,16 @@ LoadUnitTestCache (
>> // Determine the path for the cache file.
>>
>> // NOTE: This devpath is allocated and must be freed.
>>
>> //
>>
>> - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
>>
>> + FileName = GetCacheFileName (FrameworkHandle);
>>
>> + if (FileName == NULL) {
>>
>> + return EFI_INVALID_PARAMETER;
>>
>> + }
>>
>>
>> //
>>
>> // 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 +435,8 @@ Exit:
>> //
>>
>> // Free allocated buffers
>>
>> //
>>
>> - if (FileDevicePath != NULL) {
>>
>> - FreePool (FileDevicePath);
>>
>> + if (FileName != NULL) {
>>
>> + FreePool (FileName);
>>
>> }
>>
>>
>> if (IsFileOpened) {
>>
>> @@ -426,3 +454,57 @@ Exit:
>> *SaveData = Buffer;
>>
>> return Status;
>>
>> }
>>
>> +
>>
>> +/**
>>
>> + Shell based UnitTestPersistenceLib library constructor.
>>
>> +
>>
>> + @param[in] ImageHandle The firmware allocated handle for the EFI
>> image.
>>
>> + @param[in] SystemTable A pointer to the EFI System Table.
>>
>> +
>>
>> + @retval EFI_SUCCESS The constructor finished successfully.
>>
>> + @retval Others Error codes returned from
>> gBS->HandleProtocol.
>>
>> + **/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +UnitTestPersistenceLibConstructor (
>>
>> + IN EFI_HANDLE ImageHandle,
>>
>> + IN EFI_SYSTEM_TABLE *SystemTable
>>
>> + )
>>
>> +{
>>
>> + UINTN Index;
>>
>> + UINTN Argc;
>>
>> + CHAR16 **Argv;
>>
>> + EFI_STATUS Status;
>>
>> + EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters;
>>
>> +
>>
>> + Status = gBS->HandleProtocol (
>>
>> + gImageHandle,
>>
>> + &gEfiShellParametersProtocolGuid,
>>
>> + (VOID **)&ShellParameters
>>
>> + );
>>
>> + if (EFI_ERROR (Status)) {
>>
>> + ASSERT_EFI_ERROR (Status);
>>
>> + goto Done;
>>
>> + }
>>
>> +
>>
>> + Argc = ShellParameters->Argc;
>>
>> + Argv = ShellParameters->Argv;
>>
>> +
>>
>> + Status = EFI_SUCCESS;
>>
>> + if ((Argc > 1) && (Argv != NULL)) {
>>
>> + // This might be our cue, check for whether we need to do anything
>>
>> + for (Index = 1; Index < Argc; Index++) {
>>
>> + if (StrCmp (Argv[Index], L"--CachePath") == 0) {
>>
>> + // Need to update the potential cache path to designated path
>>
>> + if (Index < Argc - 1) {
>>
>> + mCachePath = Argv[Index + 1];
>>
>> + } else {
>>
>> + Print (L" --CachePath <Path of where to save unit test
>> cache files, i.e. FS0:TestFolder>\n");
>>
>> + }
>>
>> + }
>>
>> + }
>>
>> + }
>>
>> +
>>
>> +Done:
>>
>> + return Status;
>>
>> +}
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v3 1/1] UnitTestFrameworkPkg: UnitTestPersistenceLib: Save Unit Test Cache Option
2023-06-30 23:00 ` [edk2-devel] " Michael Kubacki
2023-07-07 15:45 ` Kun Qin
@ 2023-07-20 0:48 ` Michael D Kinney
1 sibling, 0 replies; 5+ messages in thread
From: Michael D Kinney @ 2023-07-20 0:48 UTC (permalink / raw)
To: Michael Kubacki, devel@edk2.groups.io, kuqin12@gmail.com
Cc: Sean Brogan, Kinney, Michael D
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
> -----Original Message-----
> From: Michael Kubacki <mikuback@linux.microsoft.com>
> Sent: Friday, June 30, 2023 4:00 PM
> To: devel@edk2.groups.io; kuqin12@gmail.com
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 1/1] UnitTestFrameworkPkg:
> UnitTestPersistenceLib: Save Unit Test Cache Option
>
> Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>
>
> I haven't seen Mike Kinney acknowledge the changes made in response to
> his v1 request so we'll need to get that before merging the patch.
>
> Thanks,
> Michael
>
> On 6/30/2023 2:14 PM, Kun Qin wrote:
> > 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 then added an input argument to allow user to specify
> where
> > to save such cache file through `--CachePath` shell argument to allow
> > even more flexibility.
> >
> > 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>
> > ---
> >
> > Notes:
> > v2:
> > - Adding input argument [Mike Kinney]
> > v3:
> > - No review, no change
> >
> >
> UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/Unit
> TestPersistenceLibSimpleFileSystem.c | 232 +++++++++++++-------
> > 1 file changed, 157 insertions(+), 75 deletions(-)
> >
> > diff --git
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/Un
> itTestPersistenceLibSimpleFileSystem.c
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/Un
> itTestPersistenceLibSimpleFileSystem.c
> > index b59991683f48..8f7f03646cd5 100644
> > ---
> a/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/Un
> itTestPersistenceLibSimpleFileSystem.c
> > +++
> b/UnitTestFrameworkPkg/Library/UnitTestPersistenceLibSimpleFileSystem/Un
> itTestPersistenceLibSimpleFileSystem.c
> > @@ -16,13 +16,16 @@
> > #include <Library/UefiBootServicesTableLib.h>
> >
> > #include <Library/DevicePathLib.h>
> >
> > #include <Library/ShellLib.h>
> >
> > +#include <Library/UefiLib.h>
> >
> > #include <Protocol/LoadedImage.h>
> >
> > #include <UnitTestFrameworkTypes.h>
> >
> >
> >
> > #define CACHE_FILE_SUFFIX L"_Cache.dat"
> >
> >
> >
> > +CHAR16 *mCachePath = NULL;
> >
> > +
> >
> > /**
> >
> > - 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 +34,8 @@
> >
> >
> > **/
> >
> > STATIC
> >
> > -EFI_DEVICE_PATH_PROTOCOL *
> >
> > -GetCacheFileDevicePath (
> >
> > +CHAR16 *
> >
> > +GetCacheFileName (
> >
> > IN UNIT_TEST_FRAMEWORK_HANDLE FrameworkHandle
> >
> > )
> >
> > {
> >
> > @@ -44,13 +47,11 @@ GetCacheFileDevicePath (
> > CHAR16 *TestName;
> >
> > UINTN DirectorySlashOffset;
> >
> > UINTN CacheFilePathLength;
> >
> > - EFI_DEVICE_PATH_PROTOCOL *CacheFileDevicePath;
> >
> >
> >
> > - Framework = (UNIT_TEST_FRAMEWORK *)FrameworkHandle;
> >
> > - AppPath = NULL;
> >
> > - CacheFilePath = NULL;
> >
> > - TestName = NULL;
> >
> > - CacheFileDevicePath = NULL;
> >
> > + Framework = (UNIT_TEST_FRAMEWORK *)FrameworkHandle;
> >
> > + AppPath = NULL;
> >
> > + CacheFilePath = NULL;
> >
> > + TestName = NULL;
> >
> >
> >
> > //
> >
> > // First, we need to get some information from the loaded image.
> >
> > @@ -85,35 +86,62 @@ GetCacheFileDevicePath (
> > // PathCleanUpDirectories (FileNameCopy);
> >
> > // if (PathRemoveLastItem (FileNameCopy)) {
> >
> > //
> >
> > - AppPath = ConvertDevicePathToText (LoadedImage-
> >FilePath, TRUE, TRUE); // NOTE: This must be freed.
> >
> > - DirectorySlashOffset = StrLen (AppPath);
> >
> > - //
> >
> > - // Make sure we didn't get any weird data.
> >
> > - //
> >
> > - if (DirectorySlashOffset == 0) {
> >
> > - DEBUG ((DEBUG_ERROR, "%a - Weird 0-length string when processing
> app path.\n", __func__));
> >
> > - goto Exit;
> >
> > - }
> >
> > + if (mCachePath == NULL) {
> >
> > + AppPath = ConvertDevicePathToText (LoadedImage->FilePath, TRUE,
> TRUE); // NOTE: This must be freed.
> >
> > + if (AppPath == NULL) {
> >
> > + goto Exit;
> >
> > + }
> >
> >
> >
> > - //
> >
> > - // Now that we know we have a decent string, let's take a deeper
> look.
> >
> > - //
> >
> > - do {
> >
> > - if (AppPath[DirectorySlashOffset] == L'\\') {
> >
> > - break;
> >
> > + DirectorySlashOffset = StrLen (AppPath);
> >
> > + //
> >
> > + // Make sure we didn't get any weird data.
> >
> > + //
> >
> > + if (DirectorySlashOffset == 0) {
> >
> > + DEBUG ((DEBUG_ERROR, "%a - Weird 0-length string when
> processing app path.\n", __func__));
> >
> > + goto Exit;
> >
> > }
> >
> >
> >
> > - DirectorySlashOffset--;
> >
> > - } while (DirectorySlashOffset > 0);
> >
> > + //
> >
> > + // Now that we know we have a decent string, let's take a deeper
> look.
> >
> > + //
> >
> > + do {
> >
> > + if (AppPath[DirectorySlashOffset] == L'\\') {
> >
> > + break;
> >
> > + }
> >
> > +
> >
> > + DirectorySlashOffset--;
> >
> > + } while (DirectorySlashOffset > 0);
> >
> >
> >
> > - //
> >
> > - // After that little maneuver, DirectorySlashOffset should be
> pointing at the last '\' in AppString.
> >
> > - // That would be the path to the parent directory that the test app
> is executing from.
> >
> > - // Let's check and make sure that's right.
> >
> > - //
> >
> > - if (AppPath[DirectorySlashOffset] != L'\\') {
> >
> > - DEBUG ((DEBUG_ERROR, "%a - Could not find a single directory
> separator in app path.\n", __func__));
> >
> > - goto Exit;
> >
> > + //
> >
> > + // After that little maneuver, DirectorySlashOffset should be
> pointing at the last '\' in AppString.
> >
> > + // That would be the path to the parent directory that the test
> app is executing from.
> >
> > + // Let's check and make sure that's right.
> >
> > + //
> >
> > + if (AppPath[DirectorySlashOffset] != L'\\') {
> >
> > + DEBUG ((DEBUG_ERROR, "%a - Could not find a single directory
> separator in app path.\n", __func__));
> >
> > + goto Exit;
> >
> > + }
> >
> > + } else {
> >
> > + AppPath = FullyQualifyPath (mCachePath); // NOTE: This must be
> freed.
> >
> > + if (AppPath == NULL) {
> >
> > + goto Exit;
> >
> > + }
> >
> > +
> >
> > + DirectorySlashOffset = StrLen (AppPath);
> >
> > +
> >
> > + if (AppPath[DirectorySlashOffset - 1] != L'\\') {
> >
> > + // Set the slash if user did not specify it on the newly
> allocated pool
> >
> > + AppPath = ReallocatePool (
> >
> > + (DirectorySlashOffset + 1) * sizeof (CHAR16),
> >
> > + (DirectorySlashOffset + 2) * sizeof (CHAR16),
> >
> > + AppPath
> >
> > + );
> >
> > + AppPath[DirectorySlashOffset] = L'\\';
> >
> > + AppPath[DirectorySlashOffset + 1] = L'\0';
> >
> > + } else {
> >
> > + // Otherwise the user input is good enough to go, mostly
> >
> > + DirectorySlashOffset--;
> >
> > + }
> >
> > }
> >
> >
> >
> > //
> >
> > @@ -135,11 +163,6 @@ GetCacheFileDevicePath (
> > StrCatS (CacheFilePath, CacheFilePathLength, TestName);
> // Copy the base name for the test cache.
> >
> > StrCatS (CacheFilePath, CacheFilePathLength, CACHE_FILE_SUFFIX);
> // Copy the file suffix.
> >
> >
> >
> > - //
> >
> > - // Finally, try to create the device path for the thing thing.
> >
> > - //
> >
> > - CacheFileDevicePath = FileDevicePath (LoadedImage->DeviceHandle,
> CacheFilePath);
> >
> > -
> >
> > Exit:
> >
> > //
> >
> > // Free allocated buffers.
> >
> > @@ -148,15 +171,11 @@ Exit:
> > FreePool (AppPath);
> >
> > }
> >
> >
> >
> > - if (CacheFilePath != NULL) {
> >
> > - FreePool (CacheFilePath);
> >
> > - }
> >
> > -
> >
> > if (TestName != NULL) {
> >
> > FreePool (TestName);
> >
> > }
> >
> >
> >
> > - return CacheFileDevicePath;
> >
> > + return CacheFilePath;
> >
> > }
> >
> >
> >
> > /**
> >
> > @@ -175,21 +194,24 @@ 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);
> >
> > + if (FileName == NULL) {
> >
> > + return FALSE;
> >
> > + }
> >
> >
> >
> > //
> >
> > // 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 +220,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 +251,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 +267,16 @@ SaveUnitTestCache (
> > // Determine the path for the cache file.
> >
> > // NOTE: This devpath is allocated and must be freed.
> >
> > //
> >
> > - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
> >
> > + FileName = GetCacheFileName (FrameworkHandle);
> >
> > + if (FileName == NULL) {
> >
> > + return EFI_INVALID_PARAMETER;
> >
> > + }
> >
> >
> >
> > //
> >
> > // 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 +295,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 +329,8 @@ SaveUnitTestCache (
> > ShellCloseFile (&FileHandle);
> >
> >
> >
> > Exit:
> >
> > - if (FileDevicePath != NULL) {
> >
> > - FreePool (FileDevicePath);
> >
> > + if (FileName != NULL) {
> >
> > + FreePool (FileName);
> >
> > }
> >
> >
> >
> > return Status;
> >
> > @@ -334,13 +359,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 +381,16 @@ LoadUnitTestCache (
> > // Determine the path for the cache file.
> >
> > // NOTE: This devpath is allocated and must be freed.
> >
> > //
> >
> > - FileDevicePath = GetCacheFileDevicePath (FrameworkHandle);
> >
> > + FileName = GetCacheFileName (FrameworkHandle);
> >
> > + if (FileName == NULL) {
> >
> > + return EFI_INVALID_PARAMETER;
> >
> > + }
> >
> >
> >
> > //
> >
> > // 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 +435,8 @@ Exit:
> > //
> >
> > // Free allocated buffers
> >
> > //
> >
> > - if (FileDevicePath != NULL) {
> >
> > - FreePool (FileDevicePath);
> >
> > + if (FileName != NULL) {
> >
> > + FreePool (FileName);
> >
> > }
> >
> >
> >
> > if (IsFileOpened) {
> >
> > @@ -426,3 +454,57 @@ Exit:
> > *SaveData = Buffer;
> >
> > return Status;
> >
> > }
> >
> > +
> >
> > +/**
> >
> > + Shell based UnitTestPersistenceLib library constructor.
> >
> > +
> >
> > + @param[in] ImageHandle The firmware allocated handle for the EFI
> image.
> >
> > + @param[in] SystemTable A pointer to the EFI System Table.
> >
> > +
> >
> > + @retval EFI_SUCCESS The constructor finished successfully.
> >
> > + @retval Others Error codes returned from gBS-
> >HandleProtocol.
> >
> > + **/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +UnitTestPersistenceLibConstructor (
> >
> > + IN EFI_HANDLE ImageHandle,
> >
> > + IN EFI_SYSTEM_TABLE *SystemTable
> >
> > + )
> >
> > +{
> >
> > + UINTN Index;
> >
> > + UINTN Argc;
> >
> > + CHAR16 **Argv;
> >
> > + EFI_STATUS Status;
> >
> > + EFI_SHELL_PARAMETERS_PROTOCOL *ShellParameters;
> >
> > +
> >
> > + Status = gBS->HandleProtocol (
> >
> > + gImageHandle,
> >
> > + &gEfiShellParametersProtocolGuid,
> >
> > + (VOID **)&ShellParameters
> >
> > + );
> >
> > + if (EFI_ERROR (Status)) {
> >
> > + ASSERT_EFI_ERROR (Status);
> >
> > + goto Done;
> >
> > + }
> >
> > +
> >
> > + Argc = ShellParameters->Argc;
> >
> > + Argv = ShellParameters->Argv;
> >
> > +
> >
> > + Status = EFI_SUCCESS;
> >
> > + if ((Argc > 1) && (Argv != NULL)) {
> >
> > + // This might be our cue, check for whether we need to do
> anything
> >
> > + for (Index = 1; Index < Argc; Index++) {
> >
> > + if (StrCmp (Argv[Index], L"--CachePath") == 0) {
> >
> > + // Need to update the potential cache path to designated path
> >
> > + if (Index < Argc - 1) {
> >
> > + mCachePath = Argv[Index + 1];
> >
> > + } else {
> >
> > + Print (L" --CachePath <Path of where to save unit test
> cache files, i.e. FS0:TestFolder>\n");
> >
> > + }
> >
> > + }
> >
> > + }
> >
> > + }
> >
> > +
> >
> > +Done:
> >
> > + return Status;
> >
> > +}
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107077): https://edk2.groups.io/g/devel/message/107077
Mute This Topic: https://groups.io/mt/99878682/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 5+ messages in thread