Re: [PATCH 1/2] ShellPkg-Shell App: Provide fully-qualified path to shell scripts

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

On 10/25/2018 12:35 AM, Jim.Dailey@dell.com wrote:
> Add a function to return the fully-qualified version of some path.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jim Dailey <jim_dailey@dell.com>
> ---
>   ShellPkg/Include/Library/ShellLib.h          | 40 +++++++++
>   ShellPkg/Library/UefiShellLib/UefiShellLib.c | 93 +++++++++++++++++++-
>   2 files changed, 132 insertions(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h
> index 92fddc50f5..cd7e9c47c3 100644
> --- a/ShellPkg/Include/Library/ShellLib.h
> +++ b/ShellPkg/Include/Library/ShellLib.h
> @@ -2,6 +2,7 @@
>     Provides interface to shell functionality for shell commands and applications.
>   
>     Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright 2018 Dell Technologies.<BR>
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
>     which accompanies this distribution.  The full text of the license may be found at
> @@ -35,6 +36,45 @@
>   extern EFI_SHELL_PARAMETERS_PROTOCOL *gEfiShellParametersProtocol;
>   extern EFI_SHELL_PROTOCOL            *gEfiShellProtocol;
>   
> +/**
> +  Return the fully-qualified version of a relative path or an absolute path that
> +  does not include a file system reference.
> +
> +  If ASSERTs are disabled, and if the input parameter is NULL or points to NULL,
> +  then NULL is returned.
> +
> +  If the input path contains a ":", this function assumes that it is part of a
> +  reference to a file system (e.g. "FS0:").  In such a case, Path is cleaned
> +  and returned.
> +
> +  If there is no working directory or there is not enough memory available to
> +  create the fully-qualified path, Path is cleaned and returned.
> +
> +  Otherwise, the current file system or working directory (as appropriate) is
> +  prepended to Path.  The input Path is freed and the resulting path is cleaned,
> +  assigned to Path, and returned.
> +
> +  NOTE: If the input path is an empty string, then the current working directory
> +  (if it exists) is returned.  In other words, an empty input path is treated
> +  exactly the same as ".".
> +
> +  @param[in, out] Path  On input, a pointer to some file or directory path.  On
> +                        output, a pointer to the clean and possibly fully-
> +                        qualified version of the input path.  The input pointer
> +                        may be freed and reassigned on output.
> +
> +  @retval NULL          The input pointer or the path itself was NULL.
> +
> +  @return A pointer to the clean, fully-qualified version of Path.  If memory
> +          allocation fails, or if there is no working directory, then a pointer
> +          to the clean, but not necessarily fully-qualified version of Path.
> +**/
> +CHAR16*
> +EFIAPI
> +FullyQualifyPath(
> +  IN OUT CHAR16     **Path
> +  );
> +
>   /**
>     This function will retrieve the information about the file for the handle
>     specified and store it in allocated pool memory.
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index f04adbb63f..52ca3ce1b1 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -2,7 +2,7 @@
>     Provides interface to shell functionality for shell commands and applications.
>   
>     (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> -  Copyright 2016 Dell Inc.
> +  Copyright 2016-2018 Dell Technologies.<BR>
>     Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
> @@ -36,6 +36,97 @@ EFI_HANDLE                    mEfiShellEnvironment2Handle;
>   FILE_HANDLE_FUNCTION_MAP      FileFunctionMap;
>   EFI_UNICODE_COLLATION_PROTOCOL  *mUnicodeCollationProtocol;
>   
> +/**
> +  Return the fully-qualified version of a relative path or an absolute path that
> +  does not include a file system reference.
> +
> +  If asserts are disabled, and if the input parameter is NULL or points to NULL,
> +  then NULL is returned.
> +
> +  If the input path contains a ":", this function assumes that it is part of a
> +  reference to a file system (e.g. "FS0:").  In such a case, Path is cleaned
> +  and returned.
> +
> +  If there is no working directory or there is not enough memory available to
> +  create the fully-qualified path, Path is cleaned and returned.
> +
> +  Otherwise, the current file system or working directory (as appropriate) is
> +  prepended to Path.  The input Path is freed and the resulting path is cleaned,
> +  assigned to Path, and returned.
> +
> +  NOTE: If the input path is an empty string, then the current working directory
> +  (if it exists) is returned.  In other words, an empty input path is treated
> +  exactly the same as ".".
> +
> +  @param[in, out] Path  On input, a pointer to some file or directory path.  On
> +                        output, a pointer to the clean and possibly fully-
> +                        qualified version of the input path.  The input pointer
> +                        may be freed and reassigned on output.
> +
> +  @retval NULL          The input pointer or the path itself was NULL.
> +
> +  @return A pointer to the clean, fully-qualified version of Path.  If memory
> +          allocation fails, or if there is no working directory, then a pointer
> +          to the clean, but not necessarily fully-qualified version of Path.
> +**/
> +CHAR16*
> +EFIAPI
> +FullyQualifyPath(
> +  IN OUT    CHAR16     **Path
This API assumes *Path is allocated in heap which may bring unnecessary 
restriction. How about we accept a CONST CHAR16 * Path, quality and 
return a new allocated string?
The parameter can be "IN CONST CHAR16 *Path".

> +  )
> +{
> +  CONST CHAR16         *WorkingPath;
> +  CHAR16               *FullyQualifiedPath;
> +  UINTN                Size;
> +
> +  ASSERT(Path != NULL);
> +  ASSERT(*Path != NULL);
> +
> +  //
> +  // Handle erroneous input when asserts are disabled.
> +  //
> +  if (Path == NULL || *Path == NULL) {
> +    return NULL;
> +  }
> +
> +  if (StrStr(*Path, L":") == NULL) {

Do we need to handle path like "fs0:a.txt"?
In Windows, it is expanded to <Current Directory of FS0> + a.txt.


> +    WorkingPath = ShellGetEnvironmentVariable(L"cwd");
> +    if (WorkingPath != NULL) {
> +      //
> +      // Room for both strings plus one more character.
> +      //
> +      Size = StrSize(WorkingPath) + StrSize(*Path);
> +      FullyQualifiedPath = AllocateZeroPool(Size);
> +      if (FullyQualifiedPath != NULL) {
> +        if (**Path == L'\\' || **Path == L'/') { > +          //
> +          // Absolute path: start with the current working directory, then
> +          // truncate the new path after the file system part.
> +          //
> +          StrCpyS(FullyQualifiedPath, Size/sizeof(CHAR16), WorkingPath);
> +          *(StrStr(FullyQualifiedPath, L":") + 1) = CHAR_NULL;
> +        } else {
> +          //
> +          // Relative path: start with the working directory and append "\".
> +          //
> +          StrCpyS(FullyQualifiedPath, Size/sizeof(CHAR16), WorkingPath);
> +          StrCatS(FullyQualifiedPath, Size/sizeof(CHAR16), L"\\");
> +        }
> +        //
> +        // Now append the absolute or relative path.
> +        //
> +        StrCatS(FullyQualifiedPath, Size/sizeof(CHAR16), *Path);
> +        FreePool(*Path);

As I mentioned early, we can leave Path as is. Do not assume it's 
allocated from heap.

> +        *Path = FullyQualifiedPath;

We can just return the FullQualifiedPath without changing Path.

> +      }
> +    }
> +  }
> +
> +  PathCleanUpDirectories(*Path);

Agree. It's to remove the potential double slash in "Relative path" 
handling and also clean up the original path.

> +
> +  return *Path;
> +}
> +
>   /**
>     Check if a Unicode character is a hexadecimal character.
>   
> 


-- 
Thanks,
Ray