Re: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle FileNames

["Liming Gao" <liming.gao@intel.com>]

Push @ 6407186db9505f101ece4e1571eceed69b9fbdbe

>-----Original Message-----
>From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>Liming Gao
>Sent: Monday, November 04, 2019 8:50 AM
>To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io;
>Marvin.Haeuser@outlook.com
>Cc: vit9696@protonmail.com; Kinney, Michael D <michael.d.kinney@intel.com>
>Subject: Re: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>Root handle FileNames
>
>Reviewed-by: Liming Gao <liming.gao@intel.com>
>
>>-----Original Message-----
>>From: Gao, Zhichao
>>Sent: Thursday, October 24, 2019 10:52 AM
>>To: devel@edk2.groups.io; Marvin.Haeuser@outlook.com
>>Cc: vit9696@protonmail.com; Kinney, Michael D
>><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>>Subject: RE: [PATCH] MdePkg/UefiFileHandleLib: Tolerate more Root handle
>>FileNames
>>
>>This patch makes sense. The patch would not affect the original logic and
>>figure out the failure if the root directory file name is '\\'.
>>Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
>>
>>> -----Original Message-----
>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>>> Marvin Häuser
>>> Sent: Sunday, October 20, 2019 8:09 PM
>>> To: devel@edk2.groups.io
>>> Cc: vit9696@protonmail.com; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>
>>> Subject: [edk2-devel] [PATCH] MdePkg/UefiFileHandleLib: Tolerate more
>>> Root handle FileNames
>>>
>>> From: Marvin Haeuser <mhaeuser@outlook.de>
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2295
>>>
>>> The current implementation of the FileHandleGetFileName() function
>>> assumes that the Root directory always has the FileName '\0'.
>>> However, the only requirement the UEFI specification defines is that a
>>> prepended '\\' must be supported to access files and folders relative to the
>>> Root directory.
>>> This patch removes this assumption and supports constructing valid paths
>>for
>>> any value of FileName for the Root Directory.
>>>
>>> In practice, this fixes compatibility issues with File System drivers that
>report
>>> '\\' as the FileName of the Root directory, which currently is both
>generating
>>> an invalid path ("\\\\") and resulting in an EFI_NOT_FOUND result from the
>>> CurrentHandle->Open() call.
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Signed-off-by: Marvin Haeuser <mhaeuser@outlook.de>
>>> ---
>>>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 25
>>> ++++++++++++++------
>>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> index 5dc893833a46..28e28e5f67d5 100644
>>> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
>>> @@ -816,10 +816,25 @@ FileHandleGetFileName (
>>>          Status = EFI_OUT_OF_RESOURCES;
>>>          break;
>>>        } else {
>>> +        //
>>> +        // Prepare to move to the parent directory.
>>> +        // Also determine whether CurrentHandle refers to the Root
>directory.
>>> +        //
>>> +        Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>>> + L"..", EFI_FILE_MODE_READ, 0);
>>>          //
>>>          // We got info... do we have a name? if yes precede the current path
>>> with it...
>>>          //
>>> -        if (StrLen (FileInfo->FileName) == 0) {
>>> +        if ((StrLen (FileInfo->FileName) == 0) || EFI_ERROR (Status)) {
>>> +          //
>>> +          // Both FileInfo->FileName being '\0' and EFI_ERROR() suggest that
>>> +          // CurrentHandle refers to the Root directory.  As this loop ensures
>>> +          // FullFileName is starting with '\\' at all times, signal success
>>> +          // and exit the loop.
>>> +          // While FileInfo->FileName could theoretically be a value other than
>>> +          // '\0' or '\\', '\\' is guaranteed to be supported by the
>>> +          // specification and hence its value can safely be ignored.
>>> +          //
>>> +          Status = EFI_SUCCESS;
>>>            if (*FullFileName == NULL) {
>>>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
>>> NULL));
>>>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0); @@
>-
>>> 837,15 +852,11 @@ FileHandleGetFileName (
>>>            FreePool(FileInfo);
>>>          }
>>>        }
>>> +
>>> +      FileHandleClose(CurrentHandle);
>>>        //
>>>        // Move to the parent directory
>>>        //
>>> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
>>> L"..", EFI_FILE_MODE_READ, 0);
>>> -      if (EFI_ERROR (Status)) {
>>> -        break;
>>> -      }
>>> -
>>> -      FileHandleClose(CurrentHandle);
>>>        CurrentHandle = NextHigherHandle;
>>>      }
>>>    } else if (Status == EFI_NOT_FOUND) {
>>> --
>>> 2.23.0.windows.1
>>>
>>>
>>>
>
>
>