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

Reply via email to