public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Vladimir Olovyannikov" <vladimir.olovyannikov@broadcom.com>
To: devel@edk2.groups.io, Ray Ni <ray.ni@intel.com>,
	Zhichao Gao <zhichao.gao@intel.com>
Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Subject: [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line
Date: Thu,  9 Jan 2020 14:06:37 -0800	[thread overview]
Message-ID: <20200109220637.28691-1-vladimir.olovyannikov@broadcom.com> (raw)

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


             reply	other threads:[~2020-01-09 22:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-09 22:06 Vladimir Olovyannikov [this message]
2020-01-10  6:02 ` [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200109220637.28691-1-vladimir.olovyannikov@broadcom.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox