Re: [PATCH v2] MdePkg/UefiFileHandleLib: Fix the Root directory determination.

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
> Häuser
> Sent: Thursday, May 17, 2018 8:42 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2] [PATCH v2] MdePkg/UefiFileHandleLib: Fix the Root directory
> determination.
> 
> 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 praxis, 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.
> 
> V2:
>   - Do not change the copyright date as requested.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> ---
>  MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c | 27
> ++++++++++++++------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> index 57aad77bc135..251cbc55edb5 100644
> --- a/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> +++ b/MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.c
> @@ -820,10 +820,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.
> +          //

Per spec, Open("..") request on root directory should return error status.
So I think to keep code simple enough, we could remove the "StrLen(..) == 0"
check.

UEFI Spec 2.7:
".."   Opens the parent directory for the current location. If the location is the
root directory the request will return an error, as there is no parent directory
for the root directory.


> +          Status = EFI_SUCCESS;
>            if (*FullFileName == NULL) {
>              ASSERT((*FullFileName == NULL && Size == 0) || (*FullFileName !=
> NULL));
>              *FullFileName = StrnCatGrowLeft(FullFileName, &Size, L"\\", 0);
> @@ -841,15 +856,11 @@ FileHandleGetFileName (
>            FreePool(FileInfo);
>          }
>        }
> -      //
> -      // Move to the parent directory
> -      //
> -      Status = CurrentHandle->Open (CurrentHandle, &NextHigherHandle,
> L"..", EFI_FILE_MODE_READ, 0);
> -      if (EFI_ERROR (Status)) {
> -        break;
> -      }
> 
>        FileHandleClose(CurrentHandle);
> +      //
> +      // Move to the parent directory
> +      //
>        CurrentHandle = NextHigherHandle;
>      }
>    } else if (Status == EFI_NOT_FOUND) {
> --
> 2.17.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel