[edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line
Gao, Zhichao
zhichao.gao at intel.com
Mon Jan 13 03:06:33 UTC 2020
Hi Vladimir,
> -----Original Message-----
> From: Vladimir Olovyannikov <vladimir.olovyannikov at broadcom.com>
> Sent: Saturday, January 11, 2020 12:33 AM
> To: Ni, Ray <ray.ni at intel.com>; devel at edk2.groups.io; Gao, Zhichao
> <zhichao.gao at 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 at intel.com>
> Sent: Thursday, January 9, 2020 10:03 PM
> To: devel at edk2.groups.io; vladimir.olovyannikov at broadcom.com; Gao,
> Zhichao <zhichao.gao at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > Vladimir Olovyannikov via Groups.Io
> > Sent: Friday, January 10, 2020 6:07 AM
> > To: devel at edk2.groups.io; Ni, Ray <ray.ni at intel.com>; Gao, Zhichao
> > <zhichao.gao at intel.com>
> > Cc: Vladimir Olovyannikov <vladimir.olovyannikov at 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 at 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
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53182): https://edk2.groups.io/g/devel/message/53182
Mute This Topic: https://groups.io/mt/69589246/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list