From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mx.groups.io with SMTP id smtpd.web10.6594.1578636180309430293 for ; Thu, 09 Jan 2020 22:03:00 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.20, mailfrom: ray.ni@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jan 2020 22:02:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,415,1571727600"; d="scan'208";a="236880095" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 09 Jan 2020 22:02:59 -0800 Received: from fmsmsx113.amr.corp.intel.com (10.18.116.7) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jan 2020 22:02:59 -0800 Received: from shsmsx106.ccr.corp.intel.com (10.239.4.159) by FMSMSX113.amr.corp.intel.com (10.18.116.7) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jan 2020 22:02:58 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.197]) by SHSMSX106.ccr.corp.intel.com ([169.254.10.139]) with mapi id 14.03.0439.000; Fri, 10 Jan 2020 14:02:57 +0800 From: "Ni, Ray" To: "devel@edk2.groups.io" , "vladimir.olovyannikov@broadcom.com" , "Gao, Zhichao" Subject: Re: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Thread-Topic: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested variables substitution from cmd line Thread-Index: AQHVxzkab5xkH8TpFEqi2LxznFQSs6fjZ9qQ Date: Fri, 10 Jan 2020 06:02:57 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5C3E6CF4@SHSMSX104.ccr.corp.intel.com> References: <20200109220637.28691-1-vladimir.olovyannikov@broadcom.com> In-Reply-To: <20200109220637.28691-1-vladimir.olovyannikov@broadcom.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: ray.ni@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 On Behalf Of Vladimir = Olovyannikov via Groups.Io > Sent: Friday, January 10, 2020 6:07 AM > To: devel@edk2.groups.io; Ni, Ray ; Gao, Zhichao > Cc: Vladimir Olovyannikov > Subject: [edk2-devel] [PATCH 1/1] ShellPkg/Application: Support nested v= ariables substitution from cmd line >=20 > The current implementation of ShellSubstituteVariables() does not allow > substitution of variables names containing another variable name(s) > between %...% >=20 > 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(). >=20 > 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=3D2452 >=20 > Signed-off-by: Vladimir Olovyannikov > Contributed-under: TianoCore Contribution Agreement 1.1 > --- > ShellPkg/Application/Shell/Shell.c | 71 ++++++++++++++++++++++++------ > 1 file changed, 57 insertions(+), 14 deletions(-) >=20 > diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/S= hell/Shell.c > index d16adae0ea30..3756a71794d1 100644 > --- a/ShellPkg/Application/Shell/Shell.c > +++ b/ShellPkg/Application/Shell/Shell.c > @@ -1510,12 +1510,16 @@ ShellConvertAlias( >=20 > /** > This function will eliminate unreplaced (and therefore non-found) env= ironment variables. > - > + If a variable exists between %...%, it will be preserved, and VarExis= ts > + 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 =3D=3D 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 gon= e. > + // 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 =3D (CHAR16 *)AllocateZeroPool((SecondPercent - FirstPe= rcent) * > + 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(SecondPercen= t + 1)); > + // > + // don't need to update the locator. both % characters are g= one. > + // > + } else { > + *VarExists =3D TRUE; > + // > + // In this case, locator needs to be updated as % characters = present. > + // > + CurrentLocator =3D SecondPercent + 1; > + } > + SHELL_FREE_NON_NULL(VarName); > } else { > CurrentLocator =3D SecondPercent + 1; > } > @@ -1581,13 +1611,18 @@ StripUnreplacedEnvironmentVariables( > If the return value is not NULL the memory must be caller freed. >=20 > @param[in] OriginalCommandLine The original command line > + @param[out] NeedExtraRun Indication that the caller needs to= repeat > + this call to convert variables as t= here are > + existing variable(s) between %..% > + present after the call. >=20 > @retval NULL An error occurred. > @return The new command line with no enviro= nment 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; >=20 > ASSERT(OriginalCommandLine !=3D NULL); > + ASSERT(NeedExtraRun !=3D NULL); >=20 > ItemSize =3D 0; > NewSize =3D StrSize(OriginalCommandLine); > CurrentScriptFile =3D ShellCommandGetCurrentScriptFile(); > Temp =3D NULL; > + *NeedExtraRun =3D FALSE; >=20 > ///@todo update this to handle the %0 - %9 for scripting only (borrow= from line 1256 area) ? ? ? >=20 > @@ -1698,7 +1735,7 @@ ShellConvertVariables ( > // > // Remove non-existent environment variables > // > - StripUnreplacedEnvironmentVariables(NewCommandLine1); > + StripUnreplacedEnvironmentVariables(NewCommandLine1, NeedExtraRun); >=20 > // > // Now cleanup any straggler intentionally ignored "%" characters > @@ -1845,12 +1882,18 @@ ShellSubstituteVariables( > ) > { > CHAR16 *NewCmdLine; > - NewCmdLine =3D ShellConvertVariables(*CmdLine); > - SHELL_FREE_NON_NULL(*CmdLine); > - if (NewCmdLine =3D=3D NULL) { > - return (EFI_OUT_OF_RESOURCES); > + BOOLEAN NeedExtraRun; > + > + NeedExtraRun =3D TRUE; > + while (NeedExtraRun) { > + NewCmdLine =3D ShellConvertVariables(*CmdLine, &NeedExtraRun); > + SHELL_FREE_NON_NULL(*CmdLine); > + if (NewCmdLine =3D=3D NULL) { > + return (EFI_OUT_OF_RESOURCES); > + } > + *CmdLine =3D NewCmdLine; > } > - *CmdLine =3D NewCmdLine; > + > return (EFI_SUCCESS); > } >=20 > -- > 2.17.1 >=20 >=20 >=20