Re: [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first location

["Carsey, Jaben" <jaben.carsey@intel.com>]

Looks good to me.  

Ray?

> -----Original Message-----
> From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com]
> Sent: Monday, March 06, 2017 10:15 AM
> To: edk2-devel@lists.01.org; Carsey, Jaben <jaben.carsey@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Subject: [PATCH] ShellPkg: Fix Shell not able to run startup.nsh from first
> location
> Importance: High
> 
> If startup.nsh is placed into first location (embedded into the firmware
> image),
> and the current directory has not been set (Internal Shell has just started),
> Shell cannot execute startup because of the bug in the DoStartupScript:
> after finding the correct path of the startup.nsh from the "First location"
> and opening the file, and getting of the file handle, the correct path is
> forgotten, and then RunScriptFile() receives just the name of the file
> (from mStartupScript ariable).
> It then attempts to check if this is a file with ShellIsFile() which fails
> with "EFI_INVALID_PARAMETER" (current directory is NULL, so it cannot get
> fully qualified file name), which causes Shell to exit and unload.
> This patch fixes the issue.
> ---
>  ShellPkg/Application/Shell/Shell.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 731ba187e4d9..4967fe598448 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1162,6 +1162,7 @@ DoStartupScript(
>    Key.UnicodeChar = CHAR_NULL;
>    Key.ScanCode    = 0;
>    FileHandle      = NULL;
> +  FileStringPath = NULL;
> 
>    if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.Startup &&
> ShellInfoObject.ShellInitSettings.FileName != NULL) {
>      //
> @@ -1228,7 +1229,6 @@ DoStartupScript(
>    //
>    MapName = ShellInfoObject.NewEfiShellProtocol-
> >GetMapFromDevicePath(&ImagePath);
>    if (MapName != NULL) {
> -    FileStringPath = NULL;
>      NewSize = 0;
>      FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, MapName, 0);
>      if (FileStringPath == NULL) {
> @@ -1242,10 +1242,10 @@ DoStartupScript(
>        PathRemoveLastItem(FileStringPath);
>        FileStringPath = StrnCatGrow(&FileStringPath, &NewSize, mStartupScript,
> 0);
>        Status = ShellInfoObject.NewEfiShellProtocol-
> >OpenFileByName(FileStringPath, &FileHandle, EFI_FILE_MODE_READ);
> -      FreePool(FileStringPath);
>      }
>    }
>    if (EFI_ERROR(Status)) {
> +    SHELL_FREE_NON_NULL (FileStringPath);
>      NamePath = FileDevicePath (NULL, mStartupScript);
>      NewPath = AppendDevicePathNode (ImagePath, NamePath);
>      FreePool(NamePath);
> @@ -1254,15 +1254,21 @@ DoStartupScript(
>      // Try the location
>      //
>      Status = InternalOpenFileDevicePath(NewPath, &FileHandle,
> EFI_FILE_MODE_READ, 0);
> -    FreePool(NewPath);
> +    if (!EFI_ERROR (Status)) {
> +      FileStringPath = gEfiShellProtocol-
> >GetFilePathFromDevicePath(NewPath);
> +      if (FileStringPath == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +      }
> +    }
> +    FreePool (NewPath);
>    }
>    //
>    // If we got a file, run it
>    //
>    if (!EFI_ERROR(Status) && FileHandle != NULL) {
> -    Status = RunScriptFile (mStartupScript, FileHandle, L"",
> ShellInfoObject.NewShellParametersProtocol);
> -    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
> +    Status = RunScriptFile (FileStringPath, FileHandle, L"",
> ShellInfoObject.NewShellParametersProtocol);
>    } else {
> +    SHELL_FREE_NON_NULL (FileStringPath);
>      FileStringPath = ShellFindFilePath(mStartupScript);
>      if (FileStringPath == NULL) {
>        //
> @@ -1272,10 +1278,13 @@ DoStartupScript(
>        ASSERT(FileHandle == NULL);
>      } else {
>        Status = RunScriptFile(FileStringPath, NULL, L"",
> ShellInfoObject.NewShellParametersProtocol);
> -      FreePool(FileStringPath);
>      }
>    }
> 
> +  SHELL_FREE_NON_NULL (FileStringPath);
> +  if (FileHandle != NULL) {
> +    ShellInfoObject.NewEfiShellProtocol->CloseFile(FileHandle);
> +  }
> 
>    return (Status);
>  }
> --
> 1.9.1