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

["Gao, Zhichao" <zhichao.gao@intel.com>]

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
> 
> 
>