* [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line @ 2020-01-09 22:06 Vladimir Olovyannikov 2020-01-10 6:02 ` [edk2-devel] " Ni, Ray 2020-01-13 0:49 ` Liming Gao 0 siblings, 2 replies; 6+ messages in thread From: Vladimir Olovyannikov @ 2020-01-09 22:06 UTC (permalink / raw) To: devel, Ray Ni, Zhichao Gao; +Cc: Vladimir Olovyannikov 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 2020-01-09 22:06 [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Vladimir Olovyannikov @ 2020-01-10 6:02 ` Ni, Ray 2020-01-10 16:32 ` Vladimir Olovyannikov 2020-01-13 0:49 ` Liming Gao 1 sibling, 1 reply; 6+ messages in thread From: Ni, Ray @ 2020-01-10 6:02 UTC (permalink / raw) To: devel@edk2.groups.io, vladimir.olovyannikov@broadcom.com, Gao, Zhichao Vladimir, Is this enhancement an extension to Shell spec or violation of Shell spec? If it's an extension, I am happy to have such feature implemented. Otherwise, we may need to discuss in uefi.org. Can you list the unit tests you have done? Thanks, Ray > -----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 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 2020-01-10 6:02 ` [edk2-devel] " Ni, Ray @ 2020-01-10 16:32 ` Vladimir Olovyannikov 2020-01-13 3:06 ` Gao, Zhichao 0 siblings, 1 reply; 6+ messages in thread From: Vladimir Olovyannikov @ 2020-01-10 16:32 UTC (permalink / raw) To: Ni, Ray, devel, Gao, Zhichao Hi Ray, As an example, let's say that you have multiple variants of serverips - each interface has its own, and each interface has a property as say interface no (eth_no). It is logical to address a serverip for an interface as %serverip%eth_no%%, and in the end get the contents of variable serverip0 (say for eth_no containing value 0). In fact, Linux bash allows doing that easily. We are doing most of our upgrade/configuration stuff via Shell scripts which is very flexible, easy and efficient. To support boot redundancy, we support multiple slots for kernel/dtbs, etc, which are addressed as %update_type_%upd_type%% where upd_type can be dtb, kernel, etc. The spec says " Environment variables can be used on the command-line by using %variable-name% where variable-name is the environment variable's name. Variable substitution is not recursive." I treat it as an extension to %var% case; %var case is not touched. What do you think? Regarding unit tests. I verified variables substitutions using %var% and %var%var1%% and %%var1%var% to make sure there would be no regression. Our scripts use variables extensively, and no issues were encountered. Please let me know if you need the scripts we are using, and I will send them to you. Which unit tests are available? Thank you, Vladimir -----Original Message----- From: Ni, Ray <ray.ni@intel.com> Sent: Thursday, January 9, 2020 10:03 PM To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com; Gao, Zhichao <zhichao.gao@intel.com> Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Vladimir, Is this enhancement an extension to Shell spec or violation of Shell spec? If it's an extension, I am happy to have such feature implemented. Otherwise, we may need to discuss in uefi.org. Can you list the unit tests you have done? Thanks, Ray > -----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 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 2020-01-10 16:32 ` Vladimir Olovyannikov @ 2020-01-13 3:06 ` Gao, Zhichao 2020-01-14 17:33 ` Vladimir Olovyannikov 0 siblings, 1 reply; 6+ messages in thread From: Gao, Zhichao @ 2020-01-13 3:06 UTC (permalink / raw) To: Vladimir Olovyannikov, Ni, Ray, devel@edk2.groups.io Hi Vladimir, > -----Original Message----- > From: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > Sent: Saturday, January 11, 2020 12:33 AM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Gao, Zhichao > <zhichao.gao@intel.com> > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested > variables substitution from cmd line > > Hi Ray, > > As an example, let's say that you have multiple variants of serverips - each > interface has its own, and each interface has a property as say interface no > (eth_no). > It is logical to address a serverip for an interface as %serverip%eth_no%%, and > in the end get the contents of variable serverip0 (say for eth_no containing value > 0). > In fact, Linux bash allows doing that easily. > We are doing most of our upgrade/configuration stuff via Shell scripts which is > very flexible, easy and efficient. > To support boot redundancy, we support multiple slots for kernel/dtbs, etc, > which are addressed as %update_type_%upd_type%% where upd_type can be > dtb, kernel, etc. > The spec says " Environment variables can be used on the command-line by > using %variable-name% where variable-name is the environment variable's > name. Variable substitution is not recursive." > I treat it as an extension to %var% case; %var case is not touched. What do you > think? "Variable substitution is not recursive" means the enviornment variable doesn't support nested. If you want to push the change, you should change the spec first. And there is one variable substitution flow figure, it should be update as well. Thanks, Zhichao > > Regarding unit tests. > I verified variables substitutions using %var% and %var%var1%% > and %%var1%var% to make sure there would be no regression. > Our scripts use variables extensively, and no issues were encountered. > Please let me know if you need the scripts we are using, and I will send them to > you. > > Which unit tests are available? > > Thank you, > Vladimir > > -----Original Message----- > From: Ni, Ray <ray.ni@intel.com> > Sent: Thursday, January 9, 2020 10:03 PM > To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com; Gao, > Zhichao <zhichao.gao@intel.com> > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested > variables substitution from cmd line > > Vladimir, > Is this enhancement an extension to Shell spec or violation of Shell spec? > If it's an extension, I am happy to have such feature implemented. > Otherwise, we may need to discuss in uefi.org. > > Can you list the unit tests you have done? > > Thanks, > Ray > > > -----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 > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 2020-01-13 3:06 ` Gao, Zhichao @ 2020-01-14 17:33 ` Vladimir Olovyannikov 0 siblings, 0 replies; 6+ messages in thread From: Vladimir Olovyannikov @ 2020-01-14 17:33 UTC (permalink / raw) To: Gao, Zhichao, Ni, Ray, devel Hi Gao, > Hi Vladimir, > > > -----Original Message----- > > From: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com> > > Sent: Saturday, January 11, 2020 12:33 AM > > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Gao, Zhichao > > <zhichao.gao@intel.com> > > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support > > nested variables substitution from cmd line > > > > Hi Ray, > > > > As an example, let's say that you have multiple variants of serverips > > - each interface has its own, and each interface has a property as say > > interface no (eth_no). > > It is logical to address a serverip for an interface as > > %serverip%eth_no%%, and in the end get the contents of variable > > serverip0 (say for eth_no containing value 0). > > In fact, Linux bash allows doing that easily. > > We are doing most of our upgrade/configuration stuff via Shell scripts > > which is very flexible, easy and efficient. > > To support boot redundancy, we support multiple slots for kernel/dtbs, > > etc, which are addressed as %update_type_%upd_type%% where > upd_type > > can be dtb, kernel, etc. > > The spec says " Environment variables can be used on the command-line > > by using %variable-name% where variable-name is the environment > > variable's name. Variable substitution is not recursive." > > I treat it as an extension to %var% case; %var case is not touched. > > What do you think? > > "Variable substitution is not recursive" means the enviornment variable > doesn't support nested. If you want to push the change, you should change > the spec first. And there is one variable substitution flow figure, it > should be > update as well. OK, thank you, I understand. There is actually an ugly workaround. Vars substitution depends on the var order. Thus, there is a way to redeclare a variable (after deletion), so it changes the order in a way that in %serverip%eth_no%% eth_no comes first, and %serverip...% comes next. Then this nesting sort of working. Another ugly (but possible) way would be do something like this: set serverip%eth_no% >v tmp This gives " serverip0 = nnn.nnn.nnn.nnn", and then have a string find/supporting app which would give anything after "= ". Still not clear why this limitation in the specs.... It is easy to do with Bash script, and for a Shell script it is just a small change... May I suggest to review the spec regarding Shell variables substitution? Thank you, Vladimir > > Thanks, > Zhichao > > > > > Regarding unit tests. > > I verified variables substitutions using %var% and %var%var1%% and > > %%var1%var% to make sure there would be no regression. > > Our scripts use variables extensively, and no issues were encountered. > > Please let me know if you need the scripts we are using, and I will > > send them to you. > > > > Which unit tests are available? > > > > Thank you, > > Vladimir > > > > -----Original Message----- > > From: Ni, Ray <ray.ni@intel.com> > > Sent: Thursday, January 9, 2020 10:03 PM > > To: devel@edk2.groups.io; vladimir.olovyannikov@broadcom.com; Gao, > > Zhichao <zhichao.gao@intel.com> > > Subject: RE: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support > > nested variables substitution from cmd line > > > > Vladimir, > > Is this enhancement an extension to Shell spec or violation of Shell > > spec? > > If it's an extension, I am happy to have such feature implemented. > > Otherwise, we may need to discuss in uefi.org. > > > > Can you list the unit tests you have done? > > > > Thanks, > > Ray > > > > > -----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 > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 2020-01-09 22:06 [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Vladimir Olovyannikov 2020-01-10 6:02 ` [edk2-devel] " Ni, Ray @ 2020-01-13 0:49 ` Liming Gao 1 sibling, 0 replies; 6+ messages in thread From: Liming Gao @ 2020-01-13 0:49 UTC (permalink / raw) To: 'devel@edk2.groups.io', 'vladimir.olovyannikov@broadcom.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 > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-01-14 17:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-01-09 22:06 [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Vladimir Olovyannikov 2020-01-10 6:02 ` [edk2-devel] " Ni, Ray 2020-01-10 16:32 ` Vladimir Olovyannikov 2020-01-13 3:06 ` Gao, Zhichao 2020-01-14 17:33 ` Vladimir Olovyannikov 2020-01-13 0:49 ` Liming Gao
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox