public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Carsey, Jaben" <jaben.carsey@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"Ni, Ray" <ray.ni@intel.com>,
	"Gao, Zhichao" <zhichao.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH 32/35] ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug
Date: Thu, 26 Sep 2019 14:58:35 +0000	[thread overview]
Message-ID: <CB6E33457884FA40993F35157061515CC36AD28F@FMSMSX103.amr.corp.intel.com> (raw)
In-Reply-To: <4fc048a2-ccbd-7f32-a51b-bf065230d3f6@redhat.com>

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

Given the time gap between now and when this library was originally written, I think that we should revisit maintaining support for EFI Shell. Do we still need to have UEFI Applications that can go between EFI and UEFI versions of the shell? Are there really many EFI Shell instances that currently developed applications need to maintain support for?

Thanks
-Jaben

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, September 26, 2019 5:47 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>
> Subject: Re: [edk2-devel] [PATCH 32/35] ShellPkg/UefiShellLib: clarify
> workaround for unfixable EdkShell bug
> 
> Jaben, Ray, Zhichao,
> 
> can one of you guys please review this patch?
> 
> Thanks
> Laszlo
> 
> On 09/17/19 21:49, Laszlo Ersek wrote:
> > 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>
> > ---
> >
> > Notes:
> >     build-tested only
> >
> >  ShellPkg/Library/UefiShellLib/UefiShellLib.c | 22 ++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > index 835d0f88ca74..9f07a58eb23d 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));
> >      //
> >


  parent reply	other threads:[~2019-09-26 14:58 UTC|newest]

Thread overview: 155+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-17 19:49 [PATCH 00/35] edk2: clean up the usage of standardized (VOID*) typedefs Laszlo Ersek
2019-09-17 19:49 ` [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID Laszlo Ersek
2019-09-17 20:06   ` [edk2-devel] " Ni, Ray
2019-09-17 20:22     ` Andrew Fish
2019-09-17 20:28       ` Ni, Ray
2019-09-17 21:07         ` Andrew Fish
2019-09-17 21:11           ` Michael D Kinney
2019-09-18  8:41       ` Laszlo Ersek
2019-09-18 15:16         ` Michael D Kinney
2019-09-18 17:41           ` Laszlo Ersek
2019-09-18 15:55         ` Andrew Fish
2019-09-18 16:16           ` Leif Lindholm
2019-09-18 17:45           ` Laszlo Ersek
2019-09-18  8:36     ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 02/35] EmbeddedPkg: add missing EFIAPI calling convention specifiers Laszlo Ersek
2019-09-18 17:41   ` Leif Lindholm
2019-09-17 19:49 ` [PATCH 03/35] EmbeddedPkg/AndroidFastbootTransportTcpDxe: fix DestroyChild() call Laszlo Ersek
2019-09-24 10:47   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:12   ` Laszlo Ersek
2019-09-26 12:16   ` Ard Biesheuvel
2019-09-17 19:49 ` [PATCH 04/35] EmbeddedPkg/Universal/MmcDxe: "fix" CloseProtocol() call in BindingStop() Laszlo Ersek
2019-09-25 15:52   ` Ard Biesheuvel
2019-09-17 19:49 ` [PATCH 05/35] EmulatorPkg/DxeTimerLib: drop superfluous cast Laszlo Ersek
2019-09-17 20:02   ` Ni, Ray
2019-09-20 15:00   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 06/35] EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration Laszlo Ersek
2019-09-17 20:01   ` Ni, Ray
2019-09-24 10:44   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 07/35] MdeModulePkg: fix cast in GetModuleInfoFromHandle() calls Laszlo Ersek
2019-09-19  1:46   ` Dandan Bi
2019-09-24 10:43   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 08/35] MdeModulePkg/UefiHiiLib: stop using EFI_HANDLE in place of EFI_HII_HANDLE Laszlo Ersek
2019-09-19  1:46   ` Dandan Bi
2019-09-24 10:49   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 09/35] MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration Laszlo Ersek
2019-09-17 20:17   ` Ni, Ray
2019-09-25 16:02   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:10     ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 10/35] MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call Laszlo Ersek
2019-09-23 11:45   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-24 17:28     ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 11/35] MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI spec bug Laszlo Ersek
2019-09-19  1:47   ` Dandan Bi
2019-09-17 19:49 ` [PATCH 12/35] MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify registration Laszlo Ersek
2019-09-17 20:16   ` [edk2-devel] " Ni, Ray
2019-09-19  1:47   ` Dandan Bi
2019-09-24 10:54   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 13/35] MdeModulePkg: PEI Core: clean up "AprioriFile" handling in FindFileEx() Laszlo Ersek
2019-09-19  1:46   ` Dandan Bi
2019-09-24 15:40   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 14/35] MdeModulePkg: fix UninstallMultipleProtocolInterfaces() calls Laszlo Ersek
2019-09-17 20:16   ` Ni, Ray
2019-09-17 19:49 ` [PATCH 15/35] MdeModulePkg/PiSmmCore: make type punning consistent Laszlo Ersek
2019-09-18  2:38   ` Dong, Eric
2019-09-17 19:49 ` [PATCH 16/35] MdeModulePkg/S3SaveState: cast Position for S3BootScriptLib explicitly Laszlo Ersek
2019-09-19  1:47   ` [edk2-devel] " Dandan Bi
2019-09-17 19:49 ` [PATCH 17/35] MdePkg/DxeServicesLib: remove bogus cast Laszlo Ersek
2019-09-18  4:47   ` [edk2-devel] " Liming Gao
2019-09-24 15:38   ` Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 18/35] NetworkPkg/DxeNetLib: fix type typo in NetLibGetMacAddress() Laszlo Ersek
2019-09-24 11:00   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-10-08  0:32     ` Siyuan, Fu
2019-10-08 23:36       ` Laszlo Ersek
2019-09-26 12:14   ` Laszlo Ersek
2019-10-03 11:05     ` Laszlo Ersek
2019-10-04 19:18       ` Laszlo Ersek
2019-10-07 18:15   ` Michael D Kinney
2019-09-17 19:49 ` [PATCH 19/35] NetworkPkg: fix CloseProtocol & UninstallMultipleProtocolInterfaces calls Laszlo Ersek
2019-09-26 12:14   ` [edk2-devel] " Laszlo Ersek
2019-09-26 12:42   ` Philippe Mathieu-Daudé
2019-09-30 20:16     ` Laszlo Ersek
2019-10-01  7:18       ` Philippe Mathieu-Daudé
2019-09-27  0:03   ` Siyuan, Fu
2019-09-17 19:49 ` [PATCH 20/35] NetworkPkg/Ip4Dxe: fix NetLibDestroyServiceChild() call Laszlo Ersek
2019-09-23 16:03   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-27  0:04     ` Siyuan, Fu
2019-09-26 12:14   ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 21/35] NetworkPkg/TcpDxe: fix SockFreeFoo() parameter list Laszlo Ersek
2019-09-23 15:09   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:14   ` Laszlo Ersek
2019-09-27  0:06   ` Siyuan, Fu
2019-09-17 19:49 ` [PATCH 22/35] OvmfPkg/XenBusDxe: fix UninstallMultipleProtocolInterfaces() call Laszlo Ersek
2019-09-18  9:32   ` Anthony PERARD
2019-09-18 10:30   ` Julien Grall
2019-09-23 15:03   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 23/35] OvmfPkg/VirtioNetDxe: fix SignalEvent() call Laszlo Ersek
2019-09-20 14:59   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:16   ` Laszlo Ersek
2019-09-26 12:17   ` Ard Biesheuvel
2019-09-17 19:49 ` [PATCH 24/35] OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal functions Laszlo Ersek
2019-09-20 14:56   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-25 15:57   ` Ard Biesheuvel
2019-09-17 19:49 ` [PATCH 25/35] OvmfPkg/VideoDxe: document EFI_EDID_OVERRIDE_PROTOCOL.GetEdid() call Laszlo Ersek
2019-09-23 15:59   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:43     ` Laszlo Ersek
2019-10-04 20:01       ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls Laszlo Ersek
2019-09-23  9:55   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:45   ` Laszlo Ersek
2019-10-03 11:06     ` Laszlo Ersek
2019-10-04  0:05       ` Yao, Jiewen
2019-10-04 13:14       ` Zhang, Chao B
2019-10-04 18:15         ` Laszlo Ersek
2019-10-05 14:28       ` Zhang, Chao B
2019-10-07 18:14         ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 27/35] SecurityPkg: stop abusing EFI_EVENT for protocol notify registration Laszlo Ersek
2019-09-23  9:56   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 12:46   ` Laszlo Ersek
2019-10-03 11:06     ` Laszlo Ersek
2019-10-04  0:05       ` Yao, Jiewen
2019-10-04 13:16       ` Zhang, Chao B
2019-10-05 14:28       ` Zhang, Chao B
2019-09-17 19:49 ` [PATCH 28/35] ShellPkg/UefiShellDriver1CommandsLib: fix parameter list typo Laszlo Ersek
2019-09-24 15:44   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26  2:46   ` Gao, Zhichao
2019-09-17 19:49 ` [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE Laszlo Ersek
2019-09-25 18:04   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26 11:48     ` Laszlo Ersek
2019-09-26  2:43   ` Gao, Zhichao
2019-09-17 19:49 ` [PATCH 30/35] ShellPkg: stop taking EFI_HANDLE in place of SHELL_FILE_HANDLE Laszlo Ersek
2019-09-23  9:58   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-26  2:53   ` Gao, Zhichao
2019-09-26 12:08     ` Laszlo Ersek
2019-09-26 14:43       ` Gao, Zhichao
2019-09-30 19:52         ` Laszlo Ersek
2019-09-17 19:49 ` [PATCH 31/35] ShellPkg/UefiShellDebug1CommandsLib: fix ShellCloseFile() call Laszlo Ersek
2019-09-23 10:01   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-23 14:28     ` Carsey, Jaben
2019-09-24  1:18       ` Gao, Zhichao
2019-09-17 19:49 ` [PATCH 32/35] ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug Laszlo Ersek
2019-09-26 12:47   ` [edk2-devel] " Laszlo Ersek
2019-09-26 14:05     ` Gao, Zhichao
2019-09-26 14:58     ` Carsey, Jaben [this message]
2019-09-17 19:49 ` [PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking Laszlo Ersek
2019-09-26 12:48   ` [edk2-devel] " Laszlo Ersek
2019-10-03 11:10     ` Laszlo Ersek
2019-10-03 11:17       ` Achin Gupta
2019-10-04  0:10       ` Yao, Jiewen
2019-10-04 14:53       ` Achin Gupta
2019-09-17 19:49 ` [PATCH 34/35] UefiPayloadPkg/BlSupportPei: fix MMCONFIG assignment from XSDT Laszlo Ersek
2019-09-23  2:30   ` Guo Dong
2019-09-26 13:17   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-09-17 19:49 ` [PATCH 35/35] UefiPayloadPkg/BlSupportDxe: fix ReserveResourceInGcd() calls Laszlo Ersek
2019-09-23  2:28   ` [edk2-devel] " Guo Dong
2019-09-23 15:08   ` Philippe Mathieu-Daudé
2019-09-23 16:02     ` Guo Dong
2019-09-23 16:04       ` Philippe Mathieu-Daudé
2019-09-24 17:29     ` Laszlo Ersek
2019-09-19  0:32 ` [edk2-devel] [PATCH 00/35] edk2: clean up the usage of standardized (VOID*) typedefs Wu, Hao A
2019-09-23 16:27 ` Marvin Häuser
2019-09-24 20:26   ` Laszlo Ersek
2019-09-25  8:13     ` Marvin Häuser
2019-09-25 15:54       ` Laszlo Ersek
2019-10-08 23:49 ` Laszlo Ersek
2019-10-09  9:50   ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CB6E33457884FA40993F35157061515CC36AD28F@FMSMSX103.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox