Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line

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

Please remove Contributed-under: TianoCore Contribution Agreement 1.1 in the commit message, this is not required now. 

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vladimir Olovyannikov via Groups.Io
> Sent: Friday, January 10, 2020 6:07 AM
> To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Subject: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line
> 
> The current implementation of ShellSubstituteVariables() does not allow
> substitution of variables names containing another variable name(s)
> between %...%
> 
> Example: %text%var_name%% where var_name variable contains 0.
> Here the variable value which should be returned on substitution
> would be contents of text0 variable.
> The current implementation would return nothing as %text0% would be
> eliminated by StripUnreplacedEnvironmentVariables().
> 
> Modify the code so that StripUnreplacedEnvironmentVariables checks if
> variable between %...% really exists. If it does not, delete %...%.
> If it exists, preserve it and tell the caller that another run of
> ShellConvertVariables() is needed. This way, any nested variable
> between %...% can be easily retrieved.
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2452
> 
> Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  ShellPkg/Application/Shell/Shell.c | 71 ++++++++++++++++++++++++------
>  1 file changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
> index d16adae0ea30..3756a71794d1 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -1510,12 +1510,16 @@ ShellConvertAlias(
> 
>  /**
>    This function will eliminate unreplaced (and therefore non-found) environment variables.
> -
> +  If a variable exists between %...%, it will be preserved, and VarExists
> +  would be TRUE.
>    @param[in,out] CmdLine   The command line to update.
> +  @param[out] VarExists    A pointer to the BOOLEAN which is set if
> +                           a Shell variable between %...% exists.
>  **/
>  EFI_STATUS
>  StripUnreplacedEnvironmentVariables(
> -  IN OUT CHAR16 *CmdLine
> +  IN OUT CHAR16 *CmdLine,
> +  OUT BOOLEAN   *VarExists
>    )
>  {
>    CHAR16 *FirstPercent;
> @@ -1558,12 +1562,38 @@ StripUnreplacedEnvironmentVariables(
>      if (FirstQuote == NULL || SecondPercent < FirstQuote) {
>        if (IsValidEnvironmentVariableName(FirstPercent, SecondPercent)) {
>          //
> -        // We need to remove from FirstPercent to SecondPercent
> -        //
> -        CopyMem(FirstPercent, SecondPercent + 1, StrSize(SecondPercent + 1));
> -        //
> -        // don't need to update the locator.  both % characters are gone.
> +        // If this Shell variable exists, consider that another run is needed.
> +        // For example, consider a variable %var%var2%% when var2 is 0.
> +        // After the first run, it becomes %var0%, and after the second run
> +        // it contains the value of var0 variable.
> +        // Eliminate variable if it does not exist.
>          //
> +        CHAR16 *VarName;
> +
> +        // Consider NULL terminator as well
> +        VarName = (CHAR16 *)AllocateZeroPool((SecondPercent - FirstPercent) *
> +                                             sizeof(CHAR16));
> +        if (VarName) {
> +          CopyMem(VarName, FirstPercent + 1,
> +                  (SecondPercent - FirstPercent - 1) * sizeof(CHAR16));
> +        }
> +
> +        if (!VarName || !ShellGetEnvironmentVariable(VarName)) {
> +          //
> +          // We need to remove from FirstPercent to SecondPercent.
> +          //
> +          CopyMem(FirstPercent, SecondPercent + 1, StrSize(SecondPercent + 1));
> +          //
> +          // don't need to update the locator.  both % characters are gone.
> +          //
> +        } else {
> +          *VarExists = TRUE;
> +          //
> +          // In this case, locator needs to be updated as % characters present.
> +          //
> +          CurrentLocator = SecondPercent + 1;
> +        }
> +        SHELL_FREE_NON_NULL(VarName);
>        } else {
>          CurrentLocator = SecondPercent + 1;
>        }
> @@ -1581,13 +1611,18 @@ StripUnreplacedEnvironmentVariables(
>    If the return value is not NULL the memory must be caller freed.
> 
>    @param[in] OriginalCommandLine    The original command line
> +  @param[out] NeedExtraRun          Indication that the caller needs to repeat
> +                                    this call to convert variables as there are
> +                                    existing variable(s) between %..%
> +                                    present after the call.
> 
>    @retval NULL                      An error occurred.
>    @return                           The new command line with no environment variables present.
>  **/
>  CHAR16*
>  ShellConvertVariables (
> -  IN CONST CHAR16 *OriginalCommandLine
> +  IN CONST CHAR16 *OriginalCommandLine,
> +  OUT BOOLEAN     *NeedExtraRun
>    )
>  {
>    CONST CHAR16        *MasterEnvList;
> @@ -1601,11 +1636,13 @@ ShellConvertVariables (
>    ALIAS_LIST          *AliasListNode;
> 
>    ASSERT(OriginalCommandLine != NULL);
> +  ASSERT(NeedExtraRun != NULL);
> 
>    ItemSize          = 0;
>    NewSize           = StrSize(OriginalCommandLine);
>    CurrentScriptFile = ShellCommandGetCurrentScriptFile();
>    Temp              = NULL;
> +  *NeedExtraRun     = FALSE;
> 
>    ///@todo update this to handle the %0 - %9 for scripting only (borrow from line 1256 area) ? ? ?
> 
> @@ -1698,7 +1735,7 @@ ShellConvertVariables (
>    //
>    // Remove non-existent environment variables
>    //
> -  StripUnreplacedEnvironmentVariables(NewCommandLine1);
> +  StripUnreplacedEnvironmentVariables(NewCommandLine1, NeedExtraRun);
> 
>    //
>    // Now cleanup any straggler intentionally ignored "%" characters
> @@ -1845,12 +1882,18 @@ ShellSubstituteVariables(
>    )
>  {
>    CHAR16      *NewCmdLine;
> -  NewCmdLine = ShellConvertVariables(*CmdLine);
> -  SHELL_FREE_NON_NULL(*CmdLine);
> -  if (NewCmdLine == NULL) {
> -    return (EFI_OUT_OF_RESOURCES);
> +  BOOLEAN     NeedExtraRun;
> +
> +  NeedExtraRun = TRUE;
> +  while (NeedExtraRun) {
> +    NewCmdLine = ShellConvertVariables(*CmdLine, &NeedExtraRun);
> +    SHELL_FREE_NON_NULL(*CmdLine);
> +    if (NewCmdLine == NULL) {
> +      return (EFI_OUT_OF_RESOURCES);
> +    }
> +    *CmdLine = NewCmdLine;
>    }
> -  *CmdLine = NewCmdLine;
> +
>    return (EFI_SUCCESS);
>  }
> 
> --
> 2.17.1
> 
> 
>