[edk2] ShellPkg/Shell: Do not mix status when executing a command

Message ID 0877601216922E4B83A7129715B5DA2BA7B7583D27@GEORGE.Emea.Arm.com
State New
Headers show

Commit Message

Olivier Martin May 13, 2014, 12:06 a.m.
Dear ShellPkg maintainer,

please find the attached patch that fixes the following error.

    The function InternalShellExecuteDevicePath() did not differentiate
    an error occuring during the preparation of an image and an error
    occuring during its execution.

    A use case of the issue was when a EFI application was called
    in a EFI Shell script. If the EFI application was returning an
    error then the NSH script stopped its execution.
    While the EFI Shell specification says the script should continue
    its execution (see 4.2 Error Handling).

    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: Olivier Martin <olivier.martin@arm.com>

Best Regards,
Olivier

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No:  2548782
------------------------------------------------------------------------------
"Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE
Instantly run your Selenium tests across 300+ browser/OS combos.
Get unparalleled scalability from the best Selenium testing platform available
Simple to use. Nothing to install. Get started now for free."
http://p.sf.net/sfu/SauceLabs

Patch

From 5969a3758bbed8642b4c308279c99ba94994eece Mon Sep 17 00:00:00 2001
From: Olivier Martin <olivier.martin@arm.com>
Date: Tue, 13 May 2014 00:40:00 +0100
Subject: [PATCH] ShellPkg/Shell: Do not mix status when executing a command

The function InternalShellExecuteDevicePath() did not differentiate
an error occuring during the preparation of an image and an error
occuring during its execution.

A use case of the issue was when a EFI application was called
in a EFI Shell script. If the EFI application was returning an
error then the NSH script stopped its execution.
While the EFI Shell specification says the script should continue
its execution (see 4.2 Error Handling).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Olivier Martin <olivier.martin@arm.com>
---
 ShellPkg/Application/Shell/Shell.c         |    4 +++-
 ShellPkg/Application/Shell/ShellProtocol.c |   10 +++++++---
 ShellPkg/Application/Shell/ShellProtocol.h |    7 ++++---
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c b/ShellPkg/Application/Shell/Shell.c
index 056d66d..951d31e 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -2231,6 +2231,7 @@  RunCommandOrFile(
 )
 {
   EFI_STATUS                Status;
+  EFI_STATUS                StartStatus;
   CHAR16                    *CommandWithPath;
   EFI_DEVICE_PATH_PROTOCOL  *DevPath;
   SHELL_STATUS              CalleeExitStatus;
@@ -2308,6 +2309,7 @@  RunCommandOrFile(
             DevPath,
             CmdLine,
             NULL,
+            &StartStatus,
             NULL,
             NULL
            );
@@ -2317,7 +2319,7 @@  RunCommandOrFile(
           if(EFI_ERROR (Status)) {
             CalleeExitStatus = (SHELL_STATUS) (Status & (~MAX_BIT));
           } else {
-            CalleeExitStatus = SHELL_SUCCESS;
+            CalleeExitStatus = (SHELL_STATUS) StartStatus;
           }
 
           //
diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index ec45594..cfe2f40 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -1386,11 +1386,13 @@  InternalShellExecuteDevicePath(
   IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath,
   IN CONST CHAR16                   *CommandLine OPTIONAL,
   IN CONST CHAR16                   **Environment OPTIONAL,
+  OUT EFI_STATUS                    *StartImageStatus OPTIONAL,
   OUT UINTN                         *ExitDataSize OPTIONAL,
   OUT CHAR16                        **ExitData OPTIONAL
   )
 {
   EFI_STATUS                    Status;
+  EFI_STATUS                    StartStatus;
   EFI_STATUS                    CleanupStatus;
   EFI_HANDLE                    NewHandle;
   EFI_LOADED_IMAGE_PROTOCOL     *LoadedImage;
@@ -1504,11 +1506,14 @@  InternalShellExecuteDevicePath(
     // now start the image, passing up exit data if the caller requested it
     //
     if (!EFI_ERROR(Status)) {
-      Status      = gBS->StartImage(
+      StartStatus      = gBS->StartImage(
                           NewHandle,
                           ExitDataSizePtr,
                           ExitData
                           );
+      if (StartImageStatus != NULL) {
+        *StartImageStatus = StartStatus;
+      }
 
       CleanupStatus = gBS->UninstallProtocolInterface(
                             NewHandle,
@@ -1620,6 +1625,7 @@  EfiShellExecute(
     DevPath,
     Temp,
     (CONST CHAR16**)Environment,
+    StatusCode,
     &ExitDataSize,
     &ExitData);
 
@@ -1644,8 +1650,6 @@  EfiShellExecute(
       }
       FreePool (ExitData);
       Status = EFI_SUCCESS;
-    } else if ((StatusCode != NULL) && !EFI_ERROR(Status)) {
-      *StatusCode = EFI_SUCCESS;
     }
 
   //
diff --git a/ShellPkg/Application/Shell/ShellProtocol.h b/ShellPkg/Application/Shell/ShellProtocol.h
index ff12327..7cc1b01 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.h
+++ b/ShellPkg/Application/Shell/ShellProtocol.h
@@ -455,10 +455,11 @@  EfiShellEnablePageBreak (
 EFI_STATUS
 EFIAPI
 InternalShellExecuteDevicePath(
-  IN CONST EFI_HANDLE *ParentImageHandle,
+  IN CONST EFI_HANDLE               *ParentImageHandle,
   IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath,
-  IN CONST CHAR16 *CommandLine OPTIONAL,
-  IN CONST CHAR16 **Environment OPTIONAL,
+  IN CONST CHAR16                   *CommandLine OPTIONAL,
+  IN CONST CHAR16                   **Environment OPTIONAL,
+  OUT EFI_STATUS                    *StartImageStatus OPTIONAL,
   OUT UINTN                         *ExitDataSize OPTIONAL,
   OUT CHAR16                        **ExitData OPTIONAL
   );
-- 
1.7.5.4