From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 6 Sep 2019 22:19:29 +0000 (+0200)
Subject: ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug
X-Git-Tag: edk2-stable201911~168
X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=7eb6160d4fdff25ca623a4006e6f93e5e81038bf

ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug

The EDK 1 Shell (available at <https://github.com/tianocore/edk-Shell>)
has a bug in its EFI_SHELL_ENVIRONMENT2.Execute() implementation that
edk2's UefiShellLib has no choice but to work around.

Improve the explanation in the code. Also, document the implicit
EFI_HANDLE -> (EFI_HANDLE*) conversion, which happens implicitly after
dereferencing ParentHandle, with an explicit cast.

In practice, this patch is a no-op.

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
---

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 835d0f88ca..9f07a58eb2 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -1291,9 +1291,27 @@ ShellExecute (
   if (mEfiShellEnvironment2 != NULL) {
     //
     // Call EFI Shell version.
-    // Due to oddity in the EFI shell we want to dereference the ParentHandle here
     //
-    CmdStatus = (mEfiShellEnvironment2->Execute(*ParentHandle,
+    // Due to an unfixable bug in the EdkShell implementation, we must
+    // dereference "ParentHandle" here:
+    //
+    // 1. The EFI shell installs the EFI_SHELL_ENVIRONMENT2 protocol,
+    //    identified by gEfiShellEnvironment2Guid.
+    // 2. The Execute() member function takes "ParentImageHandle" as first
+    //    parameter, with type (EFI_HANDLE*).
+    // 3. In the EdkShell implementation, SEnvExecute() implements the
+    //    Execute() member function. It passes "ParentImageHandle" correctly to
+    //    SEnvDoExecute().
+    // 4. SEnvDoExecute() takes the (EFI_HANDLE*), and passes it directly --
+    //    without de-referencing -- to the HandleProtocol() boot service.
+    // 5. But HandleProtocol() takes an EFI_HANDLE.
+    //
+    // Therefore we must
+    // - de-reference "ParentHandle" here, to mask the bug in
+    //   SEnvDoExecute(), and
+    // - pass the resultant EFI_HANDLE as an (EFI_HANDLE*).
+    //
+    CmdStatus = (mEfiShellEnvironment2->Execute((EFI_HANDLE *)*ParentHandle,
                                           CommandLine,
                                           Output));
     //