Re: [edk2-devel] [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
On 09/25/19 20:04, Philippe Mathieu-Daudé wrote: > On 9/17/19 9:49 PM, Laszlo Ersek wrote: >> 25 files changed, 33 insertions(+), 33 deletions(-) > I checked every case, entertaining. > Reviewed-by: Philippe Mathieu-Daude Many thanks for the effort! :) Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48086): https://edk2.groups.io/g/devel/message/48086 Mute This Topic: https://groups.io/mt/34180232/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
Reviewed-by: Zhichao Gao > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, September 18, 2019 3:49 AM > To: edk2-devel-groups-io > Cc: Carsey, Jaben ; Ni, Ray ; > Gao, Zhichao > Subject: [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of > EFI_HII_HANDLE > > The UefiShell*CommandsLib instances have constructor functions that do > something like: > > gHiiHandle = HiiAddPackages (...); > ... > ShellCommandRegisterCommandName (..., gHiiHandle, ...); > > and destructor functions that implement the following pattern: > > HiiRemovePackages (gHiiHandle); > > The -- semantic, not functional -- problem is that "gHiiHandle" is declared > with type EFI_HANDLE, and not EFI_HII_HANDLE, in all of these library > instances, even though HiiAddPackages() correctly returns EFI_HII_HANDLE, > and HiiRemovePackages() takes EFI_HII_HANDLE. > > Once we fix the type of "gHiiHandle", it causes sort of a butterfly effect, > because it is passed around widely. Track down and update all of those > locations. > > The DynamicCommand lib instances use a similar pattern, so they are > affected too. > > NOTE: in practice, this patch is a no-op, as both EFI_HII_HANDLE and > EFI_HANDLE are typedefs to (VOID*). However, we shouldn't use > EFI_HANDLE where semantically EFI_HII_HANDLE is passed around. > > Cc: Jaben Carsey > Cc: Ray Ni > Cc: Zhichao Gao > Signed-off-by: Laszlo Ersek > --- > > Notes: > tested with: > - level 1: stall, exit > - level 2: ls > - level 3: pause > - dynamic command: tftp > - handle parsing: drivers, devices, dh > > ShellPkg/Include/Library/ShellCommandLib.h > | 2 +- > ShellPkg/Include/Library/ShellLib.h > | 4 ++-- > ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h > | 4 > ++-- > ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h > | 4 > ++-- > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h > | 2 > +- > > ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands > Lib.h | 2 +- > > ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsL > ib.h | 2 +- > > ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib > .h | 2 +- > > ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib > .h | 2 +- > > ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib > .h | 2 +- > > ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm > andsLib.h | 2 +- > ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm > andsLib.h | 2 +- > ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c > | 6 > +++--- > ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c > | 6 > +++--- > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > | 2 +- > ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > | 2 +- > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > | 2 > +- > > ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands > Lib.c | 2 +- > > ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsL > ib.c | 2 +- > > ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib > .c | 2 +- > > ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib > .c | 2 +- > > ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib > .c | 2 +- > ShellPkg/Library/UefiShellLib/UefiShellLib.c > | 4 ++-- > > ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm > andsLib.c | 2 +- > ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm > andsLib.c | 2 +- > 25 files changed, 33 insertions(+), 33 deletions(-) > > diff --git a/ShellPkg/Include/Library/ShellCommandLib.h > b/ShellPkg/Include/Library/ShellCommandLib.h > index 287bc0eba7f9..63fcac82a2de 100644 > --- a/ShellPkg/Include/Library/ShellCommandLib.h > +++ b/ShellPkg/Include/Library/ShellCommandLib.h > @@ -136,7 +136,7 @@ ShellCommandRegisterCommandName ( >INUINT32 ShellMinSupportLevel, >IN CONST CHAR16 *ProfileName, >IN CONST BOOLEAN CanAffectLE, > - IN CONST EFI_HANDLE HiiHandle, > + IN CONST EFI_HII_HANDLE HiiHandle, >IN CONST EFI_STRING_ID ManFormatHelp >); > > diff --git a/ShellPkg/Include/Library/ShellLib.h > b/ShellPkg/Include/Library/ShellLib.h > index 31594796cd21..1dc41f2cc11b 100644 > --- a/ShellPkg/Include/Library/ShellLib.h > +++ b/ShellPkg/Include/Library/ShellLib.h > @@ -965,7 +965,7 @@ ShellPrintHiiEx( >IN INT32Row OPTIONAL, >IN CONST CHAR8 *Language OPTIONAL, >IN
Re: [edk2-devel] [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
On 9/17/19 9:49 PM, Laszlo Ersek wrote: > The UefiShell*CommandsLib instances have constructor functions that do > something like: > > gHiiHandle = HiiAddPackages (...); > ... > ShellCommandRegisterCommandName (..., gHiiHandle, ...); > > and destructor functions that implement the following pattern: > > HiiRemovePackages (gHiiHandle); > > The -- semantic, not functional -- problem is that "gHiiHandle" is > declared with type EFI_HANDLE, and not EFI_HII_HANDLE, in all of these > library instances, even though HiiAddPackages() correctly returns > EFI_HII_HANDLE, and HiiRemovePackages() takes EFI_HII_HANDLE. > > Once we fix the type of "gHiiHandle", it causes sort of a butterfly > effect, because it is passed around widely. Track down and update all of > those locations. > > The DynamicCommand lib instances use a similar pattern, so they are > affected too. > > NOTE: in practice, this patch is a no-op, as both EFI_HII_HANDLE and > EFI_HANDLE are typedefs to (VOID*). However, we shouldn't use EFI_HANDLE > where semantically EFI_HII_HANDLE is passed around. > > Cc: Jaben Carsey > Cc: Ray Ni > Cc: Zhichao Gao > Signed-off-by: Laszlo Ersek > --- > > Notes: > tested with: > - level 1: stall, exit > - level 2: ls > - level 3: pause > - dynamic command: tftp > - handle parsing: drivers, devices, dh > > ShellPkg/Include/Library/ShellCommandLib.h > | 2 +- > ShellPkg/Include/Library/ShellLib.h > | 4 ++-- > ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h > | 4 ++-- > ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h > | 4 ++-- > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h > | 2 +- > ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.h > | 2 +- > ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.h > | 2 +- > ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c > | 6 +++--- > ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c > | 6 +++--- > ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c > | 2 +- > ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c > | 2 +- > ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c > | 2 +- > ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellLib/UefiShellLib.c > | 4 ++-- > ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.c > | 2 +- > ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.c > | 2 +- > 25 files changed, 33 insertions(+), 33 deletions(-) > > diff --git a/ShellPkg/Include/Library/ShellCommandLib.h > b/ShellPkg/Include/Library/ShellCommandLib.h > index 287bc0eba7f9..63fcac82a2de 100644 > --- a/ShellPkg/Include/Library/ShellCommandLib.h > +++ b/ShellPkg/Include/Library/ShellCommandLib.h > @@ -136,7 +136,7 @@ ShellCommandRegisterCommandName ( >INUINT32 ShellMinSupportLevel, >IN CONST CHAR16 *ProfileName, >IN CONST BOOLEAN CanAffectLE, > - IN CONST EFI_HANDLE HiiHandle, > + IN CONST EFI_HII_HANDLE HiiHandle, Updated in UefiShellCommandLib/UefiShellCommandLib.h, OK. >IN CONST EFI_STRING_ID ManFormatHelp >); > > diff --git a/ShellPkg/Include/Library/ShellLib.h > b/ShellPkg/Include/Library/ShellLib.h > index 31594796cd21..1dc41f2cc11b 100644 > --- a/ShellPkg/Include/Library/ShellLib.h > +++ b/ShellPkg/Include/Library/ShellLib.h > @@ -965,7 +965,7 @@ ShellPrintHiiEx( >IN INT32Row OPTIONAL, >IN CONST CHAR8 *Language OPTIONAL, >IN CONST EFI_STRING_ID HiiFormatStringId, > - IN CONST EFI_HANDLE HiiFormatHandle, > + IN CONST EFI_HII_HANDLE HiiFormatHandle, OK >... >); > > @@ -1260,7 +1260,7 @@ EFIAPI > ShellPromptForResponseHii ( >IN
[edk2-devel] [PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
The UefiShell*CommandsLib instances have constructor functions that do something like: gHiiHandle = HiiAddPackages (...); ... ShellCommandRegisterCommandName (..., gHiiHandle, ...); and destructor functions that implement the following pattern: HiiRemovePackages (gHiiHandle); The -- semantic, not functional -- problem is that "gHiiHandle" is declared with type EFI_HANDLE, and not EFI_HII_HANDLE, in all of these library instances, even though HiiAddPackages() correctly returns EFI_HII_HANDLE, and HiiRemovePackages() takes EFI_HII_HANDLE. Once we fix the type of "gHiiHandle", it causes sort of a butterfly effect, because it is passed around widely. Track down and update all of those locations. The DynamicCommand lib instances use a similar pattern, so they are affected too. NOTE: in practice, this patch is a no-op, as both EFI_HII_HANDLE and EFI_HANDLE are typedefs to (VOID*). However, we shouldn't use EFI_HANDLE where semantically EFI_HII_HANDLE is passed around. Cc: Jaben Carsey Cc: Ray Ni Cc: Zhichao Gao Signed-off-by: Laszlo Ersek --- Notes: tested with: - level 1: stall, exit - level 2: ls - level 3: pause - dynamic command: tftp - handle parsing: drivers, devices, dh ShellPkg/Include/Library/ShellCommandLib.h | 2 +- ShellPkg/Include/Library/ShellLib.h | 4 ++-- ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h| 4 ++-- ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h| 4 ++-- ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h | 2 +- ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.h | 2 +- ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.h | 2 +- ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.h | 2 +- ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.h | 2 +- ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.h | 2 +- ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.h | 2 +- ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.h | 2 +- ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c| 6 +++--- ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c| 6 +++--- ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 2 +- ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c | 2 +- ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c | 2 +- ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c | 2 +- ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.c | 2 +- ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.c | 2 +- ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 2 +- ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.c | 2 +- ShellPkg/Library/UefiShellLib/UefiShellLib.c | 4 ++-- ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.c | 2 +- ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.c | 2 +- 25 files changed, 33 insertions(+), 33 deletions(-) diff --git a/ShellPkg/Include/Library/ShellCommandLib.h b/ShellPkg/Include/Library/ShellCommandLib.h index 287bc0eba7f9..63fcac82a2de 100644 --- a/ShellPkg/Include/Library/ShellCommandLib.h +++ b/ShellPkg/Include/Library/ShellCommandLib.h @@ -136,7 +136,7 @@ ShellCommandRegisterCommandName ( INUINT32 ShellMinSupportLevel, IN CONST CHAR16 *ProfileName, IN CONST BOOLEAN CanAffectLE, - IN CONST EFI_HANDLE HiiHandle, + IN CONST EFI_HII_HANDLE HiiHandle, IN CONST EFI_STRING_ID ManFormatHelp ); diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h index 31594796cd21..1dc41f2cc11b 100644 --- a/ShellPkg/Include/Library/ShellLib.h +++ b/ShellPkg/Include/Library/ShellLib.h @@ -965,7 +965,7 @@ ShellPrintHiiEx( IN INT32Row OPTIONAL, IN CONST CHAR8 *Language OPTIONAL, IN CONST EFI_STRING_ID HiiFormatStringId, - IN CONST EFI_HANDLE HiiFormatHandle, + IN CONST EFI_HII_HANDLE HiiFormatHandle, ... ); @@ -1260,7 +1260,7 @@ EFIAPI ShellPromptForResponseHii ( IN SHELL_PROMPT_REQUEST_TYPE Type, IN CONST EFI_STRING_ID HiiFormatStringId, - IN CONST EFI_HANDLE HiiFormatHandle, + IN CONST EFI_HII_HANDLE HiiFormatHandle, IN OUT VOID **Response ); diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h index
