On 03/16/16 10:06, Ard Biesheuvel wrote: > On 15 March 2016 at 14:17, Qiu Shumin <[email protected]> wrote: >> This patch makes Shell support -nonesting invocation option. This option >> specifies that EFI_SHELL_PROTOCOL.Execute API nesting of a new Shell >> instance is optional and dependent on the 'nonesting' Shell environment >> variable. >> >> Difference with previous patch: >> Do not use "negative" functions names. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jaben Carsey <[email protected]> >> Reviewed-by: Qiu Shumin <[email protected]> >> Reviewed-by: Jim Dailey <[email protected]> > > > This patch breaks the build for GCC > > 08:25:14 > /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/ShellPkg/Application/Shell/ShellProtocol.c: > In function 'EfiShellExecute': > 08:25:14 > /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/ShellPkg/Application/Shell/ShellProtocol.c:1741:14: > error: variable 'CalleeStatusCode' set but not used > [-Werror=unused-but-set-variable] > 08:25:14 EFI_STATUS CalleeStatusCode; > > Please fix
Can you please fix it and push it at once? You have my R-b in advance: Reviewed-by: Laszlo Ersek <[email protected]> Thanks Laszlo > > Regards, > Ard. > > >> --- >> ShellPkg/Application/Shell/Shell.c | 41 ++++++++- >> ShellPkg/Application/Shell/Shell.h | 7 +- >> ShellPkg/Application/Shell/ShellProtocol.c | 139 >> ++++++++++++++++++++++++----- >> 3 files changed, 162 insertions(+), 25 deletions(-) >> >> diff --git a/ShellPkg/Application/Shell/Shell.c >> b/ShellPkg/Application/Shell/Shell.c >> index 40ae68d..bd695a4 100644 >> --- a/ShellPkg/Application/Shell/Shell.c >> +++ b/ShellPkg/Application/Shell/Shell.c >> @@ -1,7 +1,7 @@ >> /** @file >> This is THE shell (application) >> >> - Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR> >> + Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR> >> (C) Copyright 2013-2014 Hewlett-Packard Development Company, L.P.<BR> >> This program and the accompanying materials >> are licensed and made available under the terms and conditions of the BSD >> License >> @@ -33,6 +33,7 @@ SHELL_INFO ShellInfoObject = { >> 0, >> 0, >> 0, >> + 0, >> 0 >> }}, >> 0, >> @@ -69,6 +70,9 @@ SHELL_INFO ShellInfoObject = { >> STATIC CONST CHAR16 mScriptExtension[] = L".NSH"; >> STATIC CONST CHAR16 mExecutableExtensions[] = L".NSH;.EFI"; >> STATIC CONST CHAR16 mStartupScript[] = L"startup.nsh"; >> +CONST CHAR16 mNoNestingEnvVarName[] = L"nonesting"; >> +CONST CHAR16 mNoNestingTrue[] = L"True"; >> +CONST CHAR16 mNoNestingFalse[] = L"False"; >> >> /** >> Cleans off leading and trailing spaces and tabs. >> @@ -457,6 +461,29 @@ UefiMain ( >> } >> >> // >> + // Set the environment variable for nesting support >> + // >> + Size = 0; >> + TempString = NULL; >> + if (!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest) { >> + // >> + // No change. require nesting in Shell Protocol Execute() >> + // >> + StrnCatGrow(&TempString, >> + &Size, >> + L"False", >> + 0); >> + } else { >> + StrnCatGrow(&TempString, >> + &Size, >> + mNoNestingTrue, >> + 0); >> + } >> + Status = InternalEfiShellSetEnv(mNoNestingEnvVarName, TempString, TRUE); >> + SHELL_FREE_NON_NULL(TempString); >> + Size = 0; >> + >> + // >> // save the device path for the loaded image and the device path for >> the filepath (under loaded image) >> // These are where to look for the startup.nsh file >> // >> @@ -891,6 +918,7 @@ ProcessCommandLine( >> ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoVersion = FALSE; >> ShellInfoObject.ShellInitSettings.BitUnion.Bits.Delay = FALSE; >> ShellInfoObject.ShellInitSettings.BitUnion.Bits.Exit = FALSE; >> + ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest = FALSE; >> ShellInfoObject.ShellInitSettings.Delay = 5; >> >> // >> @@ -952,6 +980,13 @@ ProcessCommandLine( >> } >> else if (UnicodeCollation->StriColl ( >> UnicodeCollation, >> + L"-nonest", >> + CurrentArg >> + ) == 0) { >> + ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest = TRUE; >> + } >> + else if (UnicodeCollation->StriColl ( >> + UnicodeCollation, >> L"-delay", >> CurrentArg >> ) == 0) { >> @@ -1324,7 +1359,7 @@ AddLineToCommandHistory( >> BUFFER_LIST *Walker; >> UINT16 MaxHistoryCmdCount; >> UINT16 Count; >> - >> + >> Count = 0; >> MaxHistoryCmdCount = PcdGet16(PcdShellMaxHistoryCommandCount); >> >> @@ -2452,7 +2487,7 @@ SetupAndRunCommandOrFile( >> IN CHAR16 *CmdLine, >> IN CHAR16 *FirstParameter, >> IN EFI_SHELL_PARAMETERS_PROTOCOL *ParamProtocol, >> - OUT EFI_STATUS *CommandStatus >> + OUT EFI_STATUS *CommandStatus >> ) >> { >> EFI_STATUS Status; >> diff --git a/ShellPkg/Application/Shell/Shell.h >> b/ShellPkg/Application/Shell/Shell.h >> index a1b7276..29b36b0 100644 >> --- a/ShellPkg/Application/Shell/Shell.h >> +++ b/ShellPkg/Application/Shell/Shell.h >> @@ -58,6 +58,10 @@ >> #include "ConsoleWrappers.h" >> #include "FileHandleWrappers.h" >> >> +extern CONST CHAR16 mNoNestingEnvVarName[]; >> +extern CONST CHAR16 mNoNestingTrue[]; >> +extern CONST CHAR16 mNoNestingFalse[]; >> + >> typedef struct { >> LIST_ENTRY Link; ///< Standard linked list handler. >> SHELL_FILE_HANDLE *SplitStdOut; ///< ConsoleOut for use in the split. >> @@ -73,7 +77,8 @@ typedef struct { >> UINT32 NoMap:1; ///< Was "-nomap" found on command line. >> UINT32 NoVersion:1; ///< Was "-noversion" found on command line. >> UINT32 Delay:1; ///< Was "-delay[:n] found on command line >> - UINT32 Exit:1; ///< Was "-_exit" found on command line >> + UINT32 Exit:1; ///< Was "-_exit" found on command line >> + UINT32 NoNest:1; ///< Was "-nonest" found on command line >> UINT32 Reserved:7; ///< Extra bits >> } SHELL_BITS; >> >> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c >> b/ShellPkg/Application/Shell/ShellProtocol.c >> index 22d3987..2cb7bef 100644 >> --- a/ShellPkg/Application/Shell/ShellProtocol.c >> +++ b/ShellPkg/Application/Shell/ShellProtocol.c >> @@ -3,7 +3,7 @@ >> manipulation, and initialization of EFI_SHELL_PROTOCOL. >> >> (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR> >> - Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR> >> + Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR> >> This program and the accompanying materials >> are licensed and made available under the terms and conditions of the BSD >> License >> which accompanies this distribution. The full text of the license may be >> found at >> @@ -1588,6 +1588,109 @@ FreeAlloc: >> >> return(Status); >> } >> + >> +/** >> + internal worker function to load and run an image in the current shell. >> + >> + @param CommandLine Points to the NULL-terminated UCS-2 encoded >> string >> + containing the command line. If NULL then >> the command- >> + line will be empty. >> + @param Environment Points to a NULL-terminated array of >> environment >> + variables with the format 'x=y', where x is >> the >> + environment variable name and y is the >> value. If this >> + is NULL, then the current shell environment >> is used. >> + >> + @param[out] StartImageStatus Returned status from the command line. >> + >> + @retval EFI_SUCCESS The command executed successfully. The status >> code >> + returned by the command is pointed to by >> StatusCode. >> + @retval EFI_INVALID_PARAMETER The parameters are invalid. >> + @retval EFI_OUT_OF_RESOURCES Out of resources. >> + @retval EFI_UNSUPPORTED Nested shell invocations are not allowed. >> +**/ >> +EFI_STATUS >> +EFIAPI >> +InternalShellExecute( >> + IN CONST CHAR16 *CommandLine OPTIONAL, >> + IN CONST CHAR16 **Environment OPTIONAL, >> + OUT EFI_STATUS *StartImageStatus OPTIONAL >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_STATUS CleanupStatus; >> + LIST_ENTRY OrigEnvs; >> + >> + InitializeListHead(&OrigEnvs); >> + >> + // >> + // Save our current environment settings for later restoration if >> necessary >> + // >> + if (Environment != NULL) { >> + Status = GetEnvironmentVariableList(&OrigEnvs); >> + if (!EFI_ERROR(Status)) { >> + Status = SetEnvironmentVariables(Environment); >> + } else { >> + return Status; >> + } >> + } >> + >> + Status = RunShellCommand(CommandLine, StartImageStatus); >> + >> + // Restore environment variables >> + if (!IsListEmpty(&OrigEnvs)) { >> + CleanupStatus = SetEnvironmentVariableList(&OrigEnvs); >> + ASSERT_EFI_ERROR (CleanupStatus); >> + } >> + >> + return(Status); >> +} >> + >> +/** >> + Determine if the UEFI Shell is currently running with nesting enabled or >> disabled. >> + >> + @retval FALSE nesting is required >> + @retval other nesting is enabled >> +**/ >> +STATIC >> +BOOLEAN >> +EFIAPI >> +NestingEnabled( >> +) >> +{ >> + EFI_STATUS Status; >> + CHAR16 *Temp; >> + CHAR16 *Temp2; >> + UINTN TempSize; >> + BOOLEAN RetVal; >> + >> + RetVal = TRUE; >> + Temp = NULL; >> + Temp2 = NULL; >> + >> + if (ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest) { >> + TempSize = 0; >> + Temp = NULL; >> + Status = SHELL_GET_ENVIRONMENT_VARIABLE(mNoNestingEnvVarName, >> &TempSize, Temp); >> + if (Status == EFI_BUFFER_TOO_SMALL) { >> + Temp = AllocateZeroPool(TempSize + sizeof(CHAR16)); >> + if (Temp != NULL) { >> + Status = SHELL_GET_ENVIRONMENT_VARIABLE(mNoNestingEnvVarName, >> &TempSize, Temp); >> + } >> + } >> + Temp2 = StrnCatGrow(&Temp2, NULL, mNoNestingTrue, 0); >> + if (Temp != NULL && Temp2 != NULL && StringNoCaseCompare(&Temp, &Temp2) >> == 0) { >> + // >> + // Use the no nesting method. >> + // >> + RetVal = FALSE; >> + } >> + } >> + >> + SHELL_FREE_NON_NULL(Temp); >> + SHELL_FREE_NON_NULL(Temp2); >> + return (RetVal); >> +} >> + >> /** >> Execute the command line. >> >> @@ -1611,7 +1714,7 @@ FreeAlloc: >> variables with the format 'x=y', where x is the >> environment variable name and y is the value. >> If this >> is NULL, then the current shell environment is >> used. >> - @param StatusCode Points to the status code returned by the >> command. >> + @param StatusCode Points to the status code returned by the >> CommandLine. >> >> @retval EFI_SUCCESS The command executed successfully. The status >> code >> returned by the command is pointed to by >> StatusCode. >> @@ -1643,11 +1746,9 @@ EfiShellExecute( >> return (EFI_UNSUPPORTED); >> } >> >> - if (Environment != NULL) { >> - // If Environment isn't null, load a new image of the shell with its own >> - // environment >> + if (NestingEnabled()) { >> DevPath = AppendDevicePath (ShellInfoObject.ImageDevPath, >> ShellInfoObject.FileDevPath); >> - >> + >> DEBUG_CODE_BEGIN(); >> Temp = ConvertDevicePathToText(ShellInfoObject.FileDevPath, TRUE, TRUE); >> FreePool(Temp); >> @@ -1676,19 +1777,10 @@ EfiShellExecute( >> FreePool(DevPath); >> FreePool(Temp); >> } else { >> - // If Environment is NULL, we are free to use and mutate the current >> shell >> - // environment. This is much faster as uses much less memory. >> - >> - if (CommandLine == NULL) { >> - CommandLine = L""; >> - } >> - >> - Status = RunShellCommand (CommandLine, &CalleeStatusCode); >> - >> - // Pass up the command's exit code if the caller wants it >> - if (StatusCode != NULL) { >> - *StatusCode = (EFI_STATUS) CalleeStatusCode; >> - } >> + Status = InternalShellExecute( >> + (CONST CHAR16*)CommandLine, >> + (CONST CHAR16**)Environment, >> + StatusCode); >> } >> >> return(Status); >> @@ -2814,6 +2906,11 @@ EfiShellSetEnv( >> gUnicodeCollation, >> (CHAR16*)Name, >> L"uefiversion") == 0 >> + ||(!ShellInfoObject.ShellInitSettings.BitUnion.Bits.NoNest && >> + gUnicodeCollation->StriColl( >> + gUnicodeCollation, >> + (CHAR16*)Name, >> + (CHAR16*)mNoNestingEnvVarName) == 0) >> ){ >> return (EFI_INVALID_PARAMETER); >> } >> @@ -2827,7 +2924,7 @@ EfiShellSetEnv( >> FileSystemMapping is not NULL, it returns the current directory >> associated with the >> FileSystemMapping. In both cases, the returned name includes the file >> system >> mapping (i.e. fs0:\current-dir). >> - >> + >> Note that the current directory string should exclude the tailing >> backslash character. >> >> @param FileSystemMapping A pointer to the file system mapping. If >> NULL, >> @@ -2981,7 +3078,7 @@ EfiShellSetCurDir( >> ASSERT((MapListItem->CurrentDirectoryPath == NULL && Size == 0) || >> (MapListItem->CurrentDirectoryPath != NULL)); >> if (MapListItem->CurrentDirectoryPath != NULL) { >> >> MapListItem->CurrentDirectoryPath[StrLen(MapListItem->CurrentDirectoryPath)-1] >> = CHAR_NULL; >> - } >> + } >> } >> } else { >> // >> -- >> 2.7.1.windows.2 >> >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

