Date   
Re: [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Ni, Ray
 

Andrew,
I agree. Your solution is like "use strict;" in Perl language. (https://perldoc.perl.org/strict.html)
Maybe "STRICT_UEFI_TYPES" without "ER". I don't see any strict in today's code. 😊

Thanks,
Ray

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via Groups.Io
Sent: Tuesday, September 17, 2019 1:22 PM
To: Ni, Ray <ray.ni@...>
Cc: devel@edk2.groups.io; lersek@...; Achin Gupta <achin.gupta@...>; Anthony Perard
<anthony.perard@...>; Ard Biesheuvel <ard.biesheuvel@...>; You, Benjamin <benjamin.you@...>;
Zhang, Chao B <chao.b.zhang@...>; Bi, Dandan <dandan.bi@...>; David Woodhouse
<dwmw2@...>; Dong, Eric <eric.dong@...>; Dong, Guo <guo.dong@...>; Wu, Hao A
<hao.a.wu@...>; Carsey, Jaben <jaben.carsey@...>; Wang, Jian J <jian.j.wang@...>; Wu, Jiaxin
<jiaxin.wu@...>; Yao, Jiewen <jiewen.yao@...>; Justen, Jordan L <jordan.l.justen@...>; Julien Grall
<julien.grall@...>; Leif Lindholm <leif.lindholm@...>; Gao, Liming <liming.gao@...>; Ma, Maurice
<maurice.ma@...>; Kinney, Michael D <michael.d.kinney@...>; Fu, Siyuan <@sfu5>; Supreeth
Venkatesh <supreeth.venkatesh@...>; Gao, Zhichao <zhichao.gao@...>
Subject: Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID



On Sep 17, 2019, at 1:06 PM, Ni, Ray <ray.ni@...> wrote:

Laszlo,
Thank you very much for this work.
They are quite helpful to detect potential issues.

But without this specific patch being checked in, future break will still happen.
I don't want it to be checked in ASAP because I know that there are quite a lot of close source code that may get build
break due to this change.
Besides that, what prevent you make the decision to check in the changes?
Ray,

I was thinking the same thing. Could we make this an optional feature via a #define? We could always default to the Spec
Behavior, and new projects could opt into the stricter version.

#ifndef STRICTER_UEFI_TYPES
typedef VOID *EFI_PEI_FV_HANDLE;
#else
struct EFI_PEI_FV_OBJECT;
typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;
#endif

Thanks,

Andrew Fish

Thanks,
Ray


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Achin Gupta <achin.gupta@...>; Andrew Fish <afish@...>; Anthony Perard
<anthony.perard@...>;
Ard Biesheuvel <ard.biesheuvel@...>; You, Benjamin <benjamin.you@...>; Zhang, Chao B
<chao.b.zhang@...>; Bi, Dandan <dandan.bi@...>; David Woodhouse <dwmw2@...>; Dong,
Eric
<eric.dong@...>; Dong, Guo <guo.dong@...>; Wu, Hao A <hao.a.wu@...>; Carsey, Jaben
<jaben.carsey@...>; Wang, Jian J <jian.j.wang@...>; Wu, Jiaxin <jiaxin.wu@...>; Yao, Jiewen
<jiewen.yao@...>; Justen, Jordan L <jordan.l.justen@...>; Julien Grall <julien.grall@...>; Leif
Lindholm
<leif.lindholm@...>; Gao, Liming <liming.gao@...>; Ma, Maurice <maurice.ma@...>; Kinney,
Michael
D <michael.d.kinney@...>; Ni, Ray <ray.ni@...>; Fu, Siyuan <@sfu5>; Supreeth Venkatesh
<supreeth.venkatesh@...>; Gao, Zhichao <zhichao.gao@...>
Subject: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Unfortunately, the UEFI / PI / Shell specs define a number of handle types
as pointers to VOID. This is a design mistake; those types should have
been pointers to incomplete union or structure types. Any
pointer-to-object type converts implicitly to, and from, pointer-to-void,
which prevents compilers from catching at least the following two types of
mistakes:

- mixing up one handle type with another (for example, EFI_HANDLE with
EFI_EVENT),

- getting the depth of indirection wrong (for example, mixing up
(EFI_HANDLE*) with EFI_HANDLE).

In order to root out such mistakes in the edk2 codebase, introduce
incomplete structure types with unique tags, such as:

struct EFI_FOOBAR_OBJECT;
typedef struct EFI_FOOBAR_OBJECT *EFI_FOOBAR_HANDLE;

replacing the spec mandated

typedef VOID *EFI_FOOBAR_HANDLE;

(For some types, such as:

- EFI_ACPI_HANDLE,
- EFI_EVENT,
- EFI_FONT_HANDLE,
- EFI_HANDLE,
- EFI_HII_HANDLE,
- EFI_S3_BOOT_SCRIPT_POSITION,
- SHELL_FILE_HANDLE,

we connect the actual complete type (the internal, implementation-specific
type) to the typedef. Some of these also demonstrate how the code could
have looked in practice if the specs had used proper opaque (=incomplete)
types.)

Then, unleash "build" on the package DSC files. This causes the compiler
to warn about incompatible pointer assignments, and to stop the build.

The rest of the series addresses the resultant warnings. Each patch
belongs in one of two categories:

- semantic cleanups (no functional / behavioral changes),
- actual bugfixes.

As the subject line of this patch states, this specific patch is *not*
meant to be applied. It is just a "what if" patch that temporarily
isolates the standard types from each other, the way the specs should
have, so that the compiler have more information to work with.

Cc: Achin Gupta <achin.gupta@...>
Cc: Andrew Fish <afish@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Benjamin You <benjamin.you@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: David Woodhouse <dwmw2@...>
Cc: Eric Dong <eric.dong@...>
Cc: Guo Dong <guo.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jaben Carsey <jaben.carsey@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jian Wang <jian.j.wang@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien.grall@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Liming Gao <liming.gao@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>
Cc: Siyuan Fu <@sfu5>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---
MdePkg/Include/Pi/PiPeiCis.h | 6 ++++--
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++-
MdePkg/Include/Protocol/Bis.h | 3 ++-
MdePkg/Include/Protocol/Eap.h | 3 ++-
MdePkg/Include/Protocol/HiiFont.h | 3 +--
MdePkg/Include/Protocol/MmMp.h | 3 ++-
MdePkg/Include/Protocol/S3SaveState.h | 2 +-
MdePkg/Include/Protocol/Shell.h | 3 ++-
MdePkg/Include/Protocol/UserManager.h | 9 ++++++---
MdePkg/Include/Uefi/UefiBaseType.h | 6 ++++--
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++-
MdeModulePkg/Core/Dxe/Event/Event.h | 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +-
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
17 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index d9d4ed7d413a..3e9e82b62ae9 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -18,12 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The handles of EFI FV.
///
-typedef VOID *EFI_PEI_FV_HANDLE;
+struct EFI_PEI_FV_OBJECT;
+typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;

///
/// The handles of EFI FFS.
///
-typedef VOID *EFI_PEI_FILE_HANDLE;
+struct EFI_PEI_FILE_OBJECT;
+typedef struct EFI_PEI_FILE_OBJECT *EFI_PEI_FILE_HANDLE;

///
/// Declare the forward reference data structure for EFI_PEI_SERVICE.
diff --git a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
index a8e0b24c6c8d..8a1863f3e03d 100644
--- a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
+++ b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
@@ -16,7 +16,8 @@
{ 0xeb97088e, 0xcfdf, 0x49c6, { 0xbe, 0x4b, 0xd9, 0x6, 0xa5, 0xb2, 0xe, 0x86 }}

typedef UINT32 EFI_ACPI_TABLE_VERSION;
-typedef VOID *EFI_ACPI_HANDLE;
+struct EFI_ACPI_OBJECT;
+typedef struct EFI_ACPI_OBJECT *EFI_ACPI_HANDLE;

#define EFI_ACPI_TABLE_VERSION_NONE (1 << 0)
#define EFI_ACPI_TABLE_VERSION_1_0B (1 << 1)
diff --git a/MdePkg/Include/Protocol/Bis.h b/MdePkg/Include/Protocol/Bis.h
index 2be6718f4bc2..8eca94512d03 100644
--- a/MdePkg/Include/Protocol/Bis.h
+++ b/MdePkg/Include/Protocol/Bis.h
@@ -37,7 +37,8 @@ typedef struct _EFI_BIS_PROTOCOL EFI_BIS_PROTOCOL;
//
// Basic types
//
-typedef VOID *BIS_APPLICATION_HANDLE;
+struct BIS_APPLICATION_OBJECT;
+typedef struct BIS_APPLICATION_OBJECT *BIS_APPLICATION_HANDLE;
typedef UINT16 BIS_ALG_ID;
typedef UINT32 BIS_CERT_ID;

diff --git a/MdePkg/Include/Protocol/Eap.h b/MdePkg/Include/Protocol/Eap.h
index 203d0f40b0dd..06584ef409d0 100644
--- a/MdePkg/Include/Protocol/Eap.h
+++ b/MdePkg/Include/Protocol/Eap.h
@@ -28,7 +28,8 @@ typedef struct _EFI_EAP_PROTOCOL EFI_EAP_PROTOCOL;
/// Type for the identification number assigned to the Port by the
/// System in which the Port resides.
///
-typedef VOID * EFI_PORT_HANDLE;
+struct EFI_PORT_OBJECT;
+typedef struct EFI_PORT_OBJECT *EFI_PORT_HANDLE;

///
/// EAP Authentication Method Type (RFC 3748)
diff --git a/MdePkg/Include/Protocol/HiiFont.h b/MdePkg/Include/Protocol/HiiFont.h
index 1f2e321ea4e2..450cad9ada70 100644
--- a/MdePkg/Include/Protocol/HiiFont.h
+++ b/MdePkg/Include/Protocol/HiiFont.h
@@ -19,8 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
{ 0xe9ca4775, 0x8657, 0x47fc, { 0x97, 0xe7, 0x7e, 0xd6, 0x5a, 0x8, 0x43, 0x24 } }

typedef struct _EFI_HII_FONT_PROTOCOL EFI_HII_FONT_PROTOCOL;
-
-typedef VOID *EFI_FONT_HANDLE;
+typedef LIST_ENTRY *EFI_FONT_HANDLE;

///
/// EFI_HII_OUT_FLAGS.
diff --git a/MdePkg/Include/Protocol/MmMp.h b/MdePkg/Include/Protocol/MmMp.h
index beace1386cbe..cd4e0db47e08 100644
--- a/MdePkg/Include/Protocol/MmMp.h
+++ b/MdePkg/Include/Protocol/MmMp.h
@@ -36,7 +36,8 @@
//
// Completion token
//
-typedef VOID* MM_COMPLETION;
+struct MM_COMPLETION_OBJECT;
+typedef struct MM_COMPLETION_OBJECT* MM_COMPLETION;

typedef struct {
MM_COMPLETION Completion;
diff --git a/MdePkg/Include/Protocol/S3SaveState.h b/MdePkg/Include/Protocol/S3SaveState.h
index c1b8f8b9e08d..235c36be6737 100644
--- a/MdePkg/Include/Protocol/S3SaveState.h
+++ b/MdePkg/Include/Protocol/S3SaveState.h
@@ -21,7 +21,7 @@
{ 0xe857caf6, 0xc046, 0x45dc, { 0xbe, 0x3f, 0xee, 0x7, 0x65, 0xfb, 0xa8, 0x87 }}


-typedef VOID *EFI_S3_BOOT_SCRIPT_POSITION;
+typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION;

typedef struct _EFI_S3_SAVE_STATE_PROTOCOL EFI_S3_SAVE_STATE_PROTOCOL;

diff --git a/MdePkg/Include/Protocol/Shell.h b/MdePkg/Include/Protocol/Shell.h
index cfb7878228c5..bf791792b4f2 100644
--- a/MdePkg/Include/Protocol/Shell.h
+++ b/MdePkg/Include/Protocol/Shell.h
@@ -11,12 +11,13 @@
#define __EFI_SHELL_PROTOCOL_H__

#include <Guid/FileInfo.h>
+#include <Protocol/SimpleFileSystem.h>

#define EFI_SHELL_PROTOCOL_GUID \
{ \
0x6302d008, 0x7f9b, 0x4f30, { 0x87, 0xac, 0x60, 0xc9, 0xfe, 0xf5, 0xda, 0x4e } \
}
-typedef VOID *SHELL_FILE_HANDLE;
+typedef EFI_FILE_PROTOCOL *SHELL_FILE_HANDLE;

typedef enum {
///
diff --git a/MdePkg/Include/Protocol/UserManager.h b/MdePkg/Include/Protocol/UserManager.h
index 26ac4955f1ec..9abfcffbeebf 100644
--- a/MdePkg/Include/Protocol/UserManager.h
+++ b/MdePkg/Include/Protocol/UserManager.h
@@ -24,8 +24,10 @@
0xbaf1e6de, 0x209e, 0x4adb, { 0x8d, 0x96, 0xfd, 0x8b, 0x71, 0xf3, 0xf6, 0x83 } \
}

-typedef VOID *EFI_USER_PROFILE_HANDLE;
-typedef VOID *EFI_USER_INFO_HANDLE;
+struct EFI_USER_PROFILE_OBJECT;
+typedef struct EFI_USER_PROFILE_OBJECT *EFI_USER_PROFILE_HANDLE;
+struct EFI_USER_INFO_OBJECT;
+typedef struct EFI_USER_INFO_OBJECT *EFI_USER_INFO_HANDLE;

///
/// The attributes of the user profile information.
@@ -157,7 +159,8 @@ typedef CHAR16 *EFI_USER_INFO_CREDENTIAL_PROVIDER_NAME;
/// Biometric Exchange Formats Framework) specification.
///
#define EFI_USER_INFO_CBEFF_RECORD 0x0B
-typedef VOID *EFI_USER_INFO_CBEFF;
+struct EFI_USER_INFO_CBEFF_OBJECT;
+typedef struct EFI_USER_INFO_CBEFF_OBJECT *EFI_USER_INFO_CBEFF;
///
/// Indicates how close of a match the fingerprint must be in order to be considered a match.
///
diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/UefiBaseType.h
index a62f13dd064f..be5831991b52 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -28,11 +28,13 @@ typedef RETURN_STATUS EFI_STATUS;
///
/// A collection of related interfaces.
///
-typedef VOID *EFI_HANDLE;
+struct EFI_OBJECT;
+typedef struct EFI_OBJECT *EFI_HANDLE;
///
/// Handle to an event structure.
///
-typedef VOID *EFI_EVENT;
+struct EFI_EVENT_OBJECT;
+typedef struct EFI_EVENT_OBJECT *EFI_EVENT;
///
/// Task priority level.
///
diff --git a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
index 4a1346a599d0..93bf9e9e0f13 100644
--- a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
+++ b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
@@ -20,7 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The following types are currently defined:
///
-typedef VOID* EFI_HII_HANDLE;
+struct EFI_HII_OBJECT;
+typedef struct EFI_HII_OBJECT* EFI_HII_HANDLE;
typedef CHAR16* EFI_STRING;
typedef UINT16 EFI_IMAGE_ID;
typedef UINT16 EFI_QUESTION_ID;
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.h b/MdeModulePkg/Core/Dxe/Event/Event.h
index 8141c5003eec..42590cb1dd09 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.h
+++ b/MdeModulePkg/Core/Dxe/Event/Event.h
@@ -37,7 +37,7 @@ typedef struct {
} TIMER_EVENT_INFO;

#define EVENT_SIGNATURE SIGNATURE_32('e','v','n','t')
-typedef struct {
+typedef struct EFI_EVENT_OBJECT {
UINTN Signature;
UINT32 Type;
UINT32 SignalCount;
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.h b/MdeModulePkg/Core/Dxe/Hand/Handle.h
index 83eb2b9f3afe..1f1ab3274e8a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.h
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.h
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 0908e7f4e9e7..c55da58d465e 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -145,7 +145,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
index 50d4c96edb63..bfebbb1f8182 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
@@ -88,7 +88,7 @@ struct _EFI_AML_NODE_LIST {
// This buffer should not be freed.
// Size is the total size of this ACPI node buffer.
//
-typedef struct {
+typedef struct EFI_ACPI_OBJECT {
UINT32 Signature;
UINT8 *Buffer;
UINTN Size;
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index 4a3feab94df5..48972d0fcad6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -274,7 +274,7 @@ typedef struct _HII_DATABASE_PACKAGE_LIST_INSTANCE {

#define HII_HANDLE_SIGNATURE SIGNATURE_32 ('h','i','h','l')

-typedef struct {
+typedef struct EFI_HII_OBJECT {
UINTN Signature;
LIST_ENTRY Handle;
UINTN Key;
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 4d0eed273f50..dcf91bc5e916 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -105,7 +105,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
--
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47388): https://edk2.groups.io/g/devel/message/47388
Mute This Topic: https://groups.io/mt/34180199/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@...]
-=-=-=-=-=-=

Re: [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Andrew Fish
 

On Sep 17, 2019, at 1:06 PM, Ni, Ray <ray.ni@...> wrote:

Laszlo,
Thank you very much for this work.
They are quite helpful to detect potential issues.

But without this specific patch being checked in, future break will still happen.
I don't want it to be checked in ASAP because I know that there are quite a lot of close source code that may get build break due to this change.
Besides that, what prevent you make the decision to check in the changes?
Ray,

I was thinking the same thing. Could we make this an optional feature via a #define? We could always default to the Spec Behavior, and new projects could opt into the stricter version.

#ifndef STRICTER_UEFI_TYPES
typedef VOID *EFI_PEI_FV_HANDLE;
#else
struct EFI_PEI_FV_OBJECT;
typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;
#endif

Thanks,

Andrew Fish

Thanks,
Ray


-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Achin Gupta <achin.gupta@...>; Andrew Fish <afish@...>; Anthony Perard <anthony.perard@...>;
Ard Biesheuvel <ard.biesheuvel@...>; You, Benjamin <benjamin.you@...>; Zhang, Chao B
<chao.b.zhang@...>; Bi, Dandan <dandan.bi@...>; David Woodhouse <dwmw2@...>; Dong, Eric
<eric.dong@...>; Dong, Guo <guo.dong@...>; Wu, Hao A <hao.a.wu@...>; Carsey, Jaben
<jaben.carsey@...>; Wang, Jian J <jian.j.wang@...>; Wu, Jiaxin <jiaxin.wu@...>; Yao, Jiewen
<jiewen.yao@...>; Justen, Jordan L <jordan.l.justen@...>; Julien Grall <julien.grall@...>; Leif Lindholm
<leif.lindholm@...>; Gao, Liming <liming.gao@...>; Ma, Maurice <maurice.ma@...>; Kinney, Michael
D <michael.d.kinney@...>; Ni, Ray <ray.ni@...>; Fu, Siyuan <@sfu5>; Supreeth Venkatesh
<supreeth.venkatesh@...>; Gao, Zhichao <zhichao.gao@...>
Subject: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Unfortunately, the UEFI / PI / Shell specs define a number of handle types
as pointers to VOID. This is a design mistake; those types should have
been pointers to incomplete union or structure types. Any
pointer-to-object type converts implicitly to, and from, pointer-to-void,
which prevents compilers from catching at least the following two types of
mistakes:

- mixing up one handle type with another (for example, EFI_HANDLE with
EFI_EVENT),

- getting the depth of indirection wrong (for example, mixing up
(EFI_HANDLE*) with EFI_HANDLE).

In order to root out such mistakes in the edk2 codebase, introduce
incomplete structure types with unique tags, such as:

struct EFI_FOOBAR_OBJECT;
typedef struct EFI_FOOBAR_OBJECT *EFI_FOOBAR_HANDLE;

replacing the spec mandated

typedef VOID *EFI_FOOBAR_HANDLE;

(For some types, such as:

- EFI_ACPI_HANDLE,
- EFI_EVENT,
- EFI_FONT_HANDLE,
- EFI_HANDLE,
- EFI_HII_HANDLE,
- EFI_S3_BOOT_SCRIPT_POSITION,
- SHELL_FILE_HANDLE,

we connect the actual complete type (the internal, implementation-specific
type) to the typedef. Some of these also demonstrate how the code could
have looked in practice if the specs had used proper opaque (=incomplete)
types.)

Then, unleash "build" on the package DSC files. This causes the compiler
to warn about incompatible pointer assignments, and to stop the build.

The rest of the series addresses the resultant warnings. Each patch
belongs in one of two categories:

- semantic cleanups (no functional / behavioral changes),
- actual bugfixes.

As the subject line of this patch states, this specific patch is *not*
meant to be applied. It is just a "what if" patch that temporarily
isolates the standard types from each other, the way the specs should
have, so that the compiler have more information to work with.

Cc: Achin Gupta <achin.gupta@...>
Cc: Andrew Fish <afish@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Benjamin You <benjamin.you@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: David Woodhouse <dwmw2@...>
Cc: Eric Dong <eric.dong@...>
Cc: Guo Dong <guo.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jaben Carsey <jaben.carsey@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jian Wang <jian.j.wang@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien.grall@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Liming Gao <liming.gao@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>
Cc: Siyuan Fu <@sfu5>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---
MdePkg/Include/Pi/PiPeiCis.h | 6 ++++--
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++-
MdePkg/Include/Protocol/Bis.h | 3 ++-
MdePkg/Include/Protocol/Eap.h | 3 ++-
MdePkg/Include/Protocol/HiiFont.h | 3 +--
MdePkg/Include/Protocol/MmMp.h | 3 ++-
MdePkg/Include/Protocol/S3SaveState.h | 2 +-
MdePkg/Include/Protocol/Shell.h | 3 ++-
MdePkg/Include/Protocol/UserManager.h | 9 ++++++---
MdePkg/Include/Uefi/UefiBaseType.h | 6 ++++--
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++-
MdeModulePkg/Core/Dxe/Event/Event.h | 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +-
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
17 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index d9d4ed7d413a..3e9e82b62ae9 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -18,12 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The handles of EFI FV.
///
-typedef VOID *EFI_PEI_FV_HANDLE;
+struct EFI_PEI_FV_OBJECT;
+typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;

///
/// The handles of EFI FFS.
///
-typedef VOID *EFI_PEI_FILE_HANDLE;
+struct EFI_PEI_FILE_OBJECT;
+typedef struct EFI_PEI_FILE_OBJECT *EFI_PEI_FILE_HANDLE;

///
/// Declare the forward reference data structure for EFI_PEI_SERVICE.
diff --git a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
index a8e0b24c6c8d..8a1863f3e03d 100644
--- a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
+++ b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
@@ -16,7 +16,8 @@
{ 0xeb97088e, 0xcfdf, 0x49c6, { 0xbe, 0x4b, 0xd9, 0x6, 0xa5, 0xb2, 0xe, 0x86 }}

typedef UINT32 EFI_ACPI_TABLE_VERSION;
-typedef VOID *EFI_ACPI_HANDLE;
+struct EFI_ACPI_OBJECT;
+typedef struct EFI_ACPI_OBJECT *EFI_ACPI_HANDLE;

#define EFI_ACPI_TABLE_VERSION_NONE (1 << 0)
#define EFI_ACPI_TABLE_VERSION_1_0B (1 << 1)
diff --git a/MdePkg/Include/Protocol/Bis.h b/MdePkg/Include/Protocol/Bis.h
index 2be6718f4bc2..8eca94512d03 100644
--- a/MdePkg/Include/Protocol/Bis.h
+++ b/MdePkg/Include/Protocol/Bis.h
@@ -37,7 +37,8 @@ typedef struct _EFI_BIS_PROTOCOL EFI_BIS_PROTOCOL;
//
// Basic types
//
-typedef VOID *BIS_APPLICATION_HANDLE;
+struct BIS_APPLICATION_OBJECT;
+typedef struct BIS_APPLICATION_OBJECT *BIS_APPLICATION_HANDLE;
typedef UINT16 BIS_ALG_ID;
typedef UINT32 BIS_CERT_ID;

diff --git a/MdePkg/Include/Protocol/Eap.h b/MdePkg/Include/Protocol/Eap.h
index 203d0f40b0dd..06584ef409d0 100644
--- a/MdePkg/Include/Protocol/Eap.h
+++ b/MdePkg/Include/Protocol/Eap.h
@@ -28,7 +28,8 @@ typedef struct _EFI_EAP_PROTOCOL EFI_EAP_PROTOCOL;
/// Type for the identification number assigned to the Port by the
/// System in which the Port resides.
///
-typedef VOID * EFI_PORT_HANDLE;
+struct EFI_PORT_OBJECT;
+typedef struct EFI_PORT_OBJECT *EFI_PORT_HANDLE;

///
/// EAP Authentication Method Type (RFC 3748)
diff --git a/MdePkg/Include/Protocol/HiiFont.h b/MdePkg/Include/Protocol/HiiFont.h
index 1f2e321ea4e2..450cad9ada70 100644
--- a/MdePkg/Include/Protocol/HiiFont.h
+++ b/MdePkg/Include/Protocol/HiiFont.h
@@ -19,8 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
{ 0xe9ca4775, 0x8657, 0x47fc, { 0x97, 0xe7, 0x7e, 0xd6, 0x5a, 0x8, 0x43, 0x24 } }

typedef struct _EFI_HII_FONT_PROTOCOL EFI_HII_FONT_PROTOCOL;
-
-typedef VOID *EFI_FONT_HANDLE;
+typedef LIST_ENTRY *EFI_FONT_HANDLE;

///
/// EFI_HII_OUT_FLAGS.
diff --git a/MdePkg/Include/Protocol/MmMp.h b/MdePkg/Include/Protocol/MmMp.h
index beace1386cbe..cd4e0db47e08 100644
--- a/MdePkg/Include/Protocol/MmMp.h
+++ b/MdePkg/Include/Protocol/MmMp.h
@@ -36,7 +36,8 @@
//
// Completion token
//
-typedef VOID* MM_COMPLETION;
+struct MM_COMPLETION_OBJECT;
+typedef struct MM_COMPLETION_OBJECT* MM_COMPLETION;

typedef struct {
MM_COMPLETION Completion;
diff --git a/MdePkg/Include/Protocol/S3SaveState.h b/MdePkg/Include/Protocol/S3SaveState.h
index c1b8f8b9e08d..235c36be6737 100644
--- a/MdePkg/Include/Protocol/S3SaveState.h
+++ b/MdePkg/Include/Protocol/S3SaveState.h
@@ -21,7 +21,7 @@
{ 0xe857caf6, 0xc046, 0x45dc, { 0xbe, 0x3f, 0xee, 0x7, 0x65, 0xfb, 0xa8, 0x87 }}


-typedef VOID *EFI_S3_BOOT_SCRIPT_POSITION;
+typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION;

typedef struct _EFI_S3_SAVE_STATE_PROTOCOL EFI_S3_SAVE_STATE_PROTOCOL;

diff --git a/MdePkg/Include/Protocol/Shell.h b/MdePkg/Include/Protocol/Shell.h
index cfb7878228c5..bf791792b4f2 100644
--- a/MdePkg/Include/Protocol/Shell.h
+++ b/MdePkg/Include/Protocol/Shell.h
@@ -11,12 +11,13 @@
#define __EFI_SHELL_PROTOCOL_H__

#include <Guid/FileInfo.h>
+#include <Protocol/SimpleFileSystem.h>

#define EFI_SHELL_PROTOCOL_GUID \
{ \
0x6302d008, 0x7f9b, 0x4f30, { 0x87, 0xac, 0x60, 0xc9, 0xfe, 0xf5, 0xda, 0x4e } \
}
-typedef VOID *SHELL_FILE_HANDLE;
+typedef EFI_FILE_PROTOCOL *SHELL_FILE_HANDLE;

typedef enum {
///
diff --git a/MdePkg/Include/Protocol/UserManager.h b/MdePkg/Include/Protocol/UserManager.h
index 26ac4955f1ec..9abfcffbeebf 100644
--- a/MdePkg/Include/Protocol/UserManager.h
+++ b/MdePkg/Include/Protocol/UserManager.h
@@ -24,8 +24,10 @@
0xbaf1e6de, 0x209e, 0x4adb, { 0x8d, 0x96, 0xfd, 0x8b, 0x71, 0xf3, 0xf6, 0x83 } \
}

-typedef VOID *EFI_USER_PROFILE_HANDLE;
-typedef VOID *EFI_USER_INFO_HANDLE;
+struct EFI_USER_PROFILE_OBJECT;
+typedef struct EFI_USER_PROFILE_OBJECT *EFI_USER_PROFILE_HANDLE;
+struct EFI_USER_INFO_OBJECT;
+typedef struct EFI_USER_INFO_OBJECT *EFI_USER_INFO_HANDLE;

///
/// The attributes of the user profile information.
@@ -157,7 +159,8 @@ typedef CHAR16 *EFI_USER_INFO_CREDENTIAL_PROVIDER_NAME;
/// Biometric Exchange Formats Framework) specification.
///
#define EFI_USER_INFO_CBEFF_RECORD 0x0B
-typedef VOID *EFI_USER_INFO_CBEFF;
+struct EFI_USER_INFO_CBEFF_OBJECT;
+typedef struct EFI_USER_INFO_CBEFF_OBJECT *EFI_USER_INFO_CBEFF;
///
/// Indicates how close of a match the fingerprint must be in order to be considered a match.
///
diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/UefiBaseType.h
index a62f13dd064f..be5831991b52 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -28,11 +28,13 @@ typedef RETURN_STATUS EFI_STATUS;
///
/// A collection of related interfaces.
///
-typedef VOID *EFI_HANDLE;
+struct EFI_OBJECT;
+typedef struct EFI_OBJECT *EFI_HANDLE;
///
/// Handle to an event structure.
///
-typedef VOID *EFI_EVENT;
+struct EFI_EVENT_OBJECT;
+typedef struct EFI_EVENT_OBJECT *EFI_EVENT;
///
/// Task priority level.
///
diff --git a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
index 4a1346a599d0..93bf9e9e0f13 100644
--- a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
+++ b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
@@ -20,7 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The following types are currently defined:
///
-typedef VOID* EFI_HII_HANDLE;
+struct EFI_HII_OBJECT;
+typedef struct EFI_HII_OBJECT* EFI_HII_HANDLE;
typedef CHAR16* EFI_STRING;
typedef UINT16 EFI_IMAGE_ID;
typedef UINT16 EFI_QUESTION_ID;
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.h b/MdeModulePkg/Core/Dxe/Event/Event.h
index 8141c5003eec..42590cb1dd09 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.h
+++ b/MdeModulePkg/Core/Dxe/Event/Event.h
@@ -37,7 +37,7 @@ typedef struct {
} TIMER_EVENT_INFO;

#define EVENT_SIGNATURE SIGNATURE_32('e','v','n','t')
-typedef struct {
+typedef struct EFI_EVENT_OBJECT {
UINTN Signature;
UINT32 Type;
UINT32 SignalCount;
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.h b/MdeModulePkg/Core/Dxe/Hand/Handle.h
index 83eb2b9f3afe..1f1ab3274e8a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.h
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.h
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 0908e7f4e9e7..c55da58d465e 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -145,7 +145,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
index 50d4c96edb63..bfebbb1f8182 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
@@ -88,7 +88,7 @@ struct _EFI_AML_NODE_LIST {
// This buffer should not be freed.
// Size is the total size of this ACPI node buffer.
//
-typedef struct {
+typedef struct EFI_ACPI_OBJECT {
UINT32 Signature;
UINT8 *Buffer;
UINTN Size;
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index 4a3feab94df5..48972d0fcad6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -274,7 +274,7 @@ typedef struct _HII_DATABASE_PACKAGE_LIST_INSTANCE {

#define HII_HANDLE_SIGNATURE SIGNATURE_32 ('h','i','h','l')

-typedef struct {
+typedef struct EFI_HII_OBJECT {
UINTN Signature;
LIST_ENTRY Handle;
UINTN Key;
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 4d0eed273f50..dcf91bc5e916 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -105,7 +105,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
--
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47388): https://edk2.groups.io/g/devel/message/47388
Mute This Topic: https://groups.io/mt/34180199/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@...]
-=-=-=-=-=-=

Re: [PATCH 09/35] MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wu, Hao A <hao.a.wu@...>; Wang, Jian J <jian.j.wang@...>; Gao, Liming <liming.gao@...>; Ni, Ray
<ray.ni@...>; Gao, Zhichao <zhichao.gao@...>
Subject: [PATCH 09/35] MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration

EfiCreateProtocolNotifyEvent() takes a (VOID**) for "Registration",
similarly to gBS->RegisterProtocolNotify(). We should pass the address of
an actual pointer-to-VOID, and not the address of an EFI_EVENT. EFI_EVENT
just happens to be specified as (VOID*), and has nothing to do with the
registration.

The same applies to gMmst->MmRegisterProtocolNotify().

"mFtwRegistration", "mFvRegistration", and "mFvbRegistration" are used for
nothing else.

This change is a no-op in practice; it's a semantic improvement.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <liming.gao@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
lightly tested, as these modules (except LoadFileOnFv2) are part of the
ArmVirt and/or OVMF platforms

MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c | 2 +-
MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c | 2 +-
MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c | 2 +-
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
index ae8f117905cd..de38ea028af1 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
@@ -47,7 +47,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#include <Library/UefiBootServicesTableLib.h>
#include "FaultTolerantWrite.h"
-EFI_EVENT mFvbRegistration = NULL;
+VOID *mFvbRegistration = NULL;


/**
diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index e8e935a85b5b..9612b394865b 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -56,7 +56,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include "FaultTolerantWriteSmmCommon.h"
#include <Protocol/MmEndOfDxe.h>

-EFI_EVENT mFvbRegistration = NULL;
+VOID *mFvbRegistration = NULL;
EFI_FTW_DEVICE *mFtwDevice = NULL;

///
diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
index 4af2da05e145..43fa6ce12875 100644
--- a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -36,7 +36,7 @@ typedef struct {
#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_THIS(a) CR (a, LOAD_FILE_ON_FV2_PRIVATE_DATA, LoadFile,
LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)
#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_LINK(a) CR (a, LOAD_FILE_ON_FV2_PRIVATE_DATA, Link,
LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)

-EFI_EVENT mFvRegistration;
+VOID *mFvRegistration;
LIST_ENTRY mPrivateDataList;

/**
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 3d232bb36cb4..7d2b6c8e1fad 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -13,7 +13,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

EFI_HANDLE mHandle = NULL;
EFI_EVENT mVirtualAddressChangeEvent = NULL;
-EFI_EVENT mFtwRegistration = NULL;
+VOID *mFtwRegistration = NULL;
VOID ***mVarCheckAddressPointer = NULL;
UINTN mVarCheckAddressPointerCount = 0;
EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock = { VariableLockRequestToLock };
--
2.19.1.3.g30247aa5d201

Re: [PATCH 12/35] MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify registration

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Bi, Dandan <dandan.bi@...>; Dong, Eric <eric.dong@...>; Wu, Hao A <hao.a.wu@...>; Wang, Jian J
<jian.j.wang@...>; Ni, Ray <ray.ni@...>; Gao, Zhichao <zhichao.gao@...>
Subject: [edk2-devel] [PATCH 12/35] MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify registration

EFI_REGISTER_KEYSTROKE_NOTIFY and EFI_UNREGISTER_KEYSTROKE_NOTIFY require
the notification handle to have type (VOID*). The notification handle has
nothing to do with the EFI_HANDLE type.

This change is a semantic fix; functionally, it's a no-op.

Cc: Dandan Bi <dandan.bi@...>
Cc: Eric Dong <eric.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
lightly tested: ConSplitterDxe is part of the ArmVirt and OVMF platforms

MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c | 2 +-
MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
index 63c814ae1816..9c38271b65f9 100644
--- a/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
+++ b/MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
@@ -4026,7 +4026,7 @@ ConSplitterTextInRegisterKeyNotify (
if (NewNotify == NULL) {
return EFI_OUT_OF_RESOURCES;
}
- NewNotify->NotifyHandleList = (EFI_HANDLE *) AllocateZeroPool (sizeof (EFI_HANDLE) * Private->TextInExListCount);
+ NewNotify->NotifyHandleList = (VOID **) AllocateZeroPool (sizeof (VOID *) * Private->TextInExListCount);
if (NewNotify->NotifyHandleList == NULL) {
gBS->FreePool (NewNotify);
return EFI_OUT_OF_RESOURCES;
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index 7cfd5c178861..f98797225b63 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -143,7 +143,7 @@ InternalStartMonitor(
EFI_HANDLE *Handles;
UINTN HandleCount;
UINTN HandleIndex;
- EFI_HANDLE NotifyHandle;
+ VOID *NotifyHandle;

Status = gBS->LocateHandleBuffer (
ByProtocol,
@@ -202,7 +202,7 @@ InternalStopMonitor(
EFI_KEY_DATA KeyData;
UINTN HandleCount;
UINTN HandleIndex;
- EFI_HANDLE NotifyHandle;
+ VOID *NotifyHandle;

Status = gBS->LocateHandleBuffer (
ByProtocol,
--
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47399): https://edk2.groups.io/g/devel/message/47399
Mute This Topic: https://groups.io/mt/34180213/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@...]
-=-=-=-=-=-=

Re: [PATCH 14/35] MdeModulePkg: fix UninstallMultipleProtocolInterfaces() calls

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wu, Hao A <hao.a.wu@...>; Wang, Jian J <jian.j.wang@...>; Ni, Ray <ray.ni@...>
Subject: [PATCH 14/35] MdeModulePkg: fix UninstallMultipleProtocolInterfaces() calls

Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
an (EFI_HANDLE*) as first parameter, the
UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
first parameter.

These are actual bugs. They must have remained hidden until now because
they are on error paths. Fix the UninstallMultipleProtocolInterfaces()
calls.

Cc: Hao A Wu <hao.a.wu@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c | 2 +-
MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c | 2 +-
MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 6 +++---
MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c | 2 +-
MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c | 2 +-
MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c | 2 +-
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c b/MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c
index 2b54ec51dca0..ed33a51da252 100644
--- a/MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c
+++ b/MdeModulePkg/Bus/I2c/I2cDxe/I2cBus.c
@@ -720,7 +720,7 @@ Error:

if (I2cBusContext != NULL) {
Status = gBS->UninstallMultipleProtocolInterfaces (
- &Controller,
+ Controller,
gEfiCallerIdGuid,
I2cBusContext,
NULL
diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
index c6e401176a4b..3bde96bc9576 100644
--- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
+++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
@@ -244,7 +244,7 @@ EnumerateNvmeDevNamespace (
);
if(EFI_ERROR(Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &Device->DeviceHandle,
+ Device->DeviceHandle,
&gEfiDevicePathProtocolGuid,
Device->DevicePath,
&gEfiBlockIoProtocolGuid,
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index b7832c6970ad..292dd25da817 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -313,7 +313,7 @@ RegisterPciDevice (
);
if (EFI_ERROR (Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &PciIoDevice->Handle,
+ PciIoDevice->Handle,
&gEfiDevicePathProtocolGuid,
PciIoDevice->DevicePath,
&gEfiPciIoProtocolGuid,
@@ -351,7 +351,7 @@ RegisterPciDevice (
);
if (EFI_ERROR (Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &PciIoDevice->Handle,
+ PciIoDevice->Handle,
&gEfiDevicePathProtocolGuid,
PciIoDevice->DevicePath,
&gEfiPciIoProtocolGuid,
@@ -360,7 +360,7 @@ RegisterPciDevice (
);
if (HasEfiImage) {
gBS->UninstallMultipleProtocolInterfaces (
- &PciIoDevice->Handle,
+ PciIoDevice->Handle,
&gEfiLoadFile2ProtocolGuid,
&PciIoDevice->LoadFile2,
NULL
diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
index 82db93a8b117..9fe8a482e067 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/Serial.c
@@ -665,7 +665,7 @@ CreateSerialDevice (

if (EFI_ERROR (Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &SerialDevice->Handle,
+ SerialDevice->Handle,
&gEfiDevicePathProtocolGuid, SerialDevice->DevicePath,
&gEfiSerialIoProtocolGuid, &SerialDevice->SerialIo,
NULL
diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
index 62f18d1878bd..e2ae56c5058a 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
@@ -475,7 +475,7 @@ InstallProtocolOnPartition (
);
if (EFI_ERROR (Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &Partition->Handle,
+ Partition->Handle,
&gEfiDevicePathProtocolGuid,
Partition->DevicePath,
&gEfiBlockIoProtocolGuid,
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
index eaa0d70024bb..cc0de52de411 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbEnumer.c
@@ -159,7 +159,7 @@ UsbCreateInterface (

if (EFI_ERROR (Status)) {
gBS->UninstallMultipleProtocolInterfaces (
- &UsbIf->Handle,
+ UsbIf->Handle,
&gEfiDevicePathProtocolGuid,
UsbIf->DevicePath,
&gEfiUsbIoProtocolGuid,
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
index 8c27e18cdb87..0dcbc5da2cb8 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
@@ -575,7 +575,7 @@ UsbMassInitMultiLun (
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "UsbMassInitMultiLun: OpenUsbIoProtocol By Child (%r)\n", Status));
gBS->UninstallMultipleProtocolInterfaces (
- &UsbMass->Controller,
+ UsbMass->Controller,
&gEfiDevicePathProtocolGuid,
UsbMass->DevicePath,
&gEfiBlockIoProtocolGuid,
--
2.19.1.3.g30247aa5d201

Re: [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Ni, Ray
 

Laszlo,
Thank you very much for this work.
They are quite helpful to detect potential issues.

But without this specific patch being checked in, future break will still happen.
I don't want it to be checked in ASAP because I know that there are quite a lot of close source code that may get build break due to this change.
Besides that, what prevent you make the decision to check in the changes?

Thanks,
Ray

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Achin Gupta <achin.gupta@...>; Andrew Fish <afish@...>; Anthony Perard <anthony.perard@...>;
Ard Biesheuvel <ard.biesheuvel@...>; You, Benjamin <benjamin.you@...>; Zhang, Chao B
<chao.b.zhang@...>; Bi, Dandan <dandan.bi@...>; David Woodhouse <dwmw2@...>; Dong, Eric
<eric.dong@...>; Dong, Guo <guo.dong@...>; Wu, Hao A <hao.a.wu@...>; Carsey, Jaben
<jaben.carsey@...>; Wang, Jian J <jian.j.wang@...>; Wu, Jiaxin <jiaxin.wu@...>; Yao, Jiewen
<jiewen.yao@...>; Justen, Jordan L <jordan.l.justen@...>; Julien Grall <julien.grall@...>; Leif Lindholm
<leif.lindholm@...>; Gao, Liming <liming.gao@...>; Ma, Maurice <maurice.ma@...>; Kinney, Michael
D <michael.d.kinney@...>; Ni, Ray <ray.ni@...>; Fu, Siyuan <@sfu5>; Supreeth Venkatesh
<supreeth.venkatesh@...>; Gao, Zhichao <zhichao.gao@...>
Subject: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID

Unfortunately, the UEFI / PI / Shell specs define a number of handle types
as pointers to VOID. This is a design mistake; those types should have
been pointers to incomplete union or structure types. Any
pointer-to-object type converts implicitly to, and from, pointer-to-void,
which prevents compilers from catching at least the following two types of
mistakes:

- mixing up one handle type with another (for example, EFI_HANDLE with
EFI_EVENT),

- getting the depth of indirection wrong (for example, mixing up
(EFI_HANDLE*) with EFI_HANDLE).

In order to root out such mistakes in the edk2 codebase, introduce
incomplete structure types with unique tags, such as:

struct EFI_FOOBAR_OBJECT;
typedef struct EFI_FOOBAR_OBJECT *EFI_FOOBAR_HANDLE;

replacing the spec mandated

typedef VOID *EFI_FOOBAR_HANDLE;

(For some types, such as:

- EFI_ACPI_HANDLE,
- EFI_EVENT,
- EFI_FONT_HANDLE,
- EFI_HANDLE,
- EFI_HII_HANDLE,
- EFI_S3_BOOT_SCRIPT_POSITION,
- SHELL_FILE_HANDLE,

we connect the actual complete type (the internal, implementation-specific
type) to the typedef. Some of these also demonstrate how the code could
have looked in practice if the specs had used proper opaque (=incomplete)
types.)

Then, unleash "build" on the package DSC files. This causes the compiler
to warn about incompatible pointer assignments, and to stop the build.

The rest of the series addresses the resultant warnings. Each patch
belongs in one of two categories:

- semantic cleanups (no functional / behavioral changes),
- actual bugfixes.

As the subject line of this patch states, this specific patch is *not*
meant to be applied. It is just a "what if" patch that temporarily
isolates the standard types from each other, the way the specs should
have, so that the compiler have more information to work with.

Cc: Achin Gupta <achin.gupta@...>
Cc: Andrew Fish <afish@...>
Cc: Anthony Perard <anthony.perard@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Benjamin You <benjamin.you@...>
Cc: Chao Zhang <chao.b.zhang@...>
Cc: Dandan Bi <dandan.bi@...>
Cc: David Woodhouse <dwmw2@...>
Cc: Eric Dong <eric.dong@...>
Cc: Guo Dong <guo.dong@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Jaben Carsey <jaben.carsey@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jian Wang <jian.j.wang@...>
Cc: Jiaxin Wu <jiaxin.wu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Julien Grall <julien.grall@...>
Cc: Leif Lindholm <leif.lindholm@...>
Cc: Liming Gao <liming.gao@...>
Cc: Maurice Ma <maurice.ma@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Ray Ni <ray.ni@...>
Cc: Siyuan Fu <@sfu5>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---
MdePkg/Include/Pi/PiPeiCis.h | 6 ++++--
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++-
MdePkg/Include/Protocol/Bis.h | 3 ++-
MdePkg/Include/Protocol/Eap.h | 3 ++-
MdePkg/Include/Protocol/HiiFont.h | 3 +--
MdePkg/Include/Protocol/MmMp.h | 3 ++-
MdePkg/Include/Protocol/S3SaveState.h | 2 +-
MdePkg/Include/Protocol/Shell.h | 3 ++-
MdePkg/Include/Protocol/UserManager.h | 9 ++++++---
MdePkg/Include/Uefi/UefiBaseType.h | 6 ++++--
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++-
MdeModulePkg/Core/Dxe/Event/Event.h | 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +-
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
17 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index d9d4ed7d413a..3e9e82b62ae9 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -18,12 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The handles of EFI FV.
///
-typedef VOID *EFI_PEI_FV_HANDLE;
+struct EFI_PEI_FV_OBJECT;
+typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;

///
/// The handles of EFI FFS.
///
-typedef VOID *EFI_PEI_FILE_HANDLE;
+struct EFI_PEI_FILE_OBJECT;
+typedef struct EFI_PEI_FILE_OBJECT *EFI_PEI_FILE_HANDLE;

///
/// Declare the forward reference data structure for EFI_PEI_SERVICE.
diff --git a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
index a8e0b24c6c8d..8a1863f3e03d 100644
--- a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
+++ b/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h
@@ -16,7 +16,8 @@
{ 0xeb97088e, 0xcfdf, 0x49c6, { 0xbe, 0x4b, 0xd9, 0x6, 0xa5, 0xb2, 0xe, 0x86 }}

typedef UINT32 EFI_ACPI_TABLE_VERSION;
-typedef VOID *EFI_ACPI_HANDLE;
+struct EFI_ACPI_OBJECT;
+typedef struct EFI_ACPI_OBJECT *EFI_ACPI_HANDLE;

#define EFI_ACPI_TABLE_VERSION_NONE (1 << 0)
#define EFI_ACPI_TABLE_VERSION_1_0B (1 << 1)
diff --git a/MdePkg/Include/Protocol/Bis.h b/MdePkg/Include/Protocol/Bis.h
index 2be6718f4bc2..8eca94512d03 100644
--- a/MdePkg/Include/Protocol/Bis.h
+++ b/MdePkg/Include/Protocol/Bis.h
@@ -37,7 +37,8 @@ typedef struct _EFI_BIS_PROTOCOL EFI_BIS_PROTOCOL;
//
// Basic types
//
-typedef VOID *BIS_APPLICATION_HANDLE;
+struct BIS_APPLICATION_OBJECT;
+typedef struct BIS_APPLICATION_OBJECT *BIS_APPLICATION_HANDLE;
typedef UINT16 BIS_ALG_ID;
typedef UINT32 BIS_CERT_ID;

diff --git a/MdePkg/Include/Protocol/Eap.h b/MdePkg/Include/Protocol/Eap.h
index 203d0f40b0dd..06584ef409d0 100644
--- a/MdePkg/Include/Protocol/Eap.h
+++ b/MdePkg/Include/Protocol/Eap.h
@@ -28,7 +28,8 @@ typedef struct _EFI_EAP_PROTOCOL EFI_EAP_PROTOCOL;
/// Type for the identification number assigned to the Port by the
/// System in which the Port resides.
///
-typedef VOID * EFI_PORT_HANDLE;
+struct EFI_PORT_OBJECT;
+typedef struct EFI_PORT_OBJECT *EFI_PORT_HANDLE;

///
/// EAP Authentication Method Type (RFC 3748)
diff --git a/MdePkg/Include/Protocol/HiiFont.h b/MdePkg/Include/Protocol/HiiFont.h
index 1f2e321ea4e2..450cad9ada70 100644
--- a/MdePkg/Include/Protocol/HiiFont.h
+++ b/MdePkg/Include/Protocol/HiiFont.h
@@ -19,8 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
{ 0xe9ca4775, 0x8657, 0x47fc, { 0x97, 0xe7, 0x7e, 0xd6, 0x5a, 0x8, 0x43, 0x24 } }

typedef struct _EFI_HII_FONT_PROTOCOL EFI_HII_FONT_PROTOCOL;
-
-typedef VOID *EFI_FONT_HANDLE;
+typedef LIST_ENTRY *EFI_FONT_HANDLE;

///
/// EFI_HII_OUT_FLAGS.
diff --git a/MdePkg/Include/Protocol/MmMp.h b/MdePkg/Include/Protocol/MmMp.h
index beace1386cbe..cd4e0db47e08 100644
--- a/MdePkg/Include/Protocol/MmMp.h
+++ b/MdePkg/Include/Protocol/MmMp.h
@@ -36,7 +36,8 @@
//
// Completion token
//
-typedef VOID* MM_COMPLETION;
+struct MM_COMPLETION_OBJECT;
+typedef struct MM_COMPLETION_OBJECT* MM_COMPLETION;

typedef struct {
MM_COMPLETION Completion;
diff --git a/MdePkg/Include/Protocol/S3SaveState.h b/MdePkg/Include/Protocol/S3SaveState.h
index c1b8f8b9e08d..235c36be6737 100644
--- a/MdePkg/Include/Protocol/S3SaveState.h
+++ b/MdePkg/Include/Protocol/S3SaveState.h
@@ -21,7 +21,7 @@
{ 0xe857caf6, 0xc046, 0x45dc, { 0xbe, 0x3f, 0xee, 0x7, 0x65, 0xfb, 0xa8, 0x87 }}


-typedef VOID *EFI_S3_BOOT_SCRIPT_POSITION;
+typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION;

typedef struct _EFI_S3_SAVE_STATE_PROTOCOL EFI_S3_SAVE_STATE_PROTOCOL;

diff --git a/MdePkg/Include/Protocol/Shell.h b/MdePkg/Include/Protocol/Shell.h
index cfb7878228c5..bf791792b4f2 100644
--- a/MdePkg/Include/Protocol/Shell.h
+++ b/MdePkg/Include/Protocol/Shell.h
@@ -11,12 +11,13 @@
#define __EFI_SHELL_PROTOCOL_H__

#include <Guid/FileInfo.h>
+#include <Protocol/SimpleFileSystem.h>

#define EFI_SHELL_PROTOCOL_GUID \
{ \
0x6302d008, 0x7f9b, 0x4f30, { 0x87, 0xac, 0x60, 0xc9, 0xfe, 0xf5, 0xda, 0x4e } \
}
-typedef VOID *SHELL_FILE_HANDLE;
+typedef EFI_FILE_PROTOCOL *SHELL_FILE_HANDLE;

typedef enum {
///
diff --git a/MdePkg/Include/Protocol/UserManager.h b/MdePkg/Include/Protocol/UserManager.h
index 26ac4955f1ec..9abfcffbeebf 100644
--- a/MdePkg/Include/Protocol/UserManager.h
+++ b/MdePkg/Include/Protocol/UserManager.h
@@ -24,8 +24,10 @@
0xbaf1e6de, 0x209e, 0x4adb, { 0x8d, 0x96, 0xfd, 0x8b, 0x71, 0xf3, 0xf6, 0x83 } \
}

-typedef VOID *EFI_USER_PROFILE_HANDLE;
-typedef VOID *EFI_USER_INFO_HANDLE;
+struct EFI_USER_PROFILE_OBJECT;
+typedef struct EFI_USER_PROFILE_OBJECT *EFI_USER_PROFILE_HANDLE;
+struct EFI_USER_INFO_OBJECT;
+typedef struct EFI_USER_INFO_OBJECT *EFI_USER_INFO_HANDLE;

///
/// The attributes of the user profile information.
@@ -157,7 +159,8 @@ typedef CHAR16 *EFI_USER_INFO_CREDENTIAL_PROVIDER_NAME;
/// Biometric Exchange Formats Framework) specification.
///
#define EFI_USER_INFO_CBEFF_RECORD 0x0B
-typedef VOID *EFI_USER_INFO_CBEFF;
+struct EFI_USER_INFO_CBEFF_OBJECT;
+typedef struct EFI_USER_INFO_CBEFF_OBJECT *EFI_USER_INFO_CBEFF;
///
/// Indicates how close of a match the fingerprint must be in order to be considered a match.
///
diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/UefiBaseType.h
index a62f13dd064f..be5831991b52 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -28,11 +28,13 @@ typedef RETURN_STATUS EFI_STATUS;
///
/// A collection of related interfaces.
///
-typedef VOID *EFI_HANDLE;
+struct EFI_OBJECT;
+typedef struct EFI_OBJECT *EFI_HANDLE;
///
/// Handle to an event structure.
///
-typedef VOID *EFI_EVENT;
+struct EFI_EVENT_OBJECT;
+typedef struct EFI_EVENT_OBJECT *EFI_EVENT;
///
/// Task priority level.
///
diff --git a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
index 4a1346a599d0..93bf9e9e0f13 100644
--- a/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
+++ b/MdePkg/Include/Uefi/UefiInternalFormRepresentation.h
@@ -20,7 +20,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The following types are currently defined:
///
-typedef VOID* EFI_HII_HANDLE;
+struct EFI_HII_OBJECT;
+typedef struct EFI_HII_OBJECT* EFI_HII_HANDLE;
typedef CHAR16* EFI_STRING;
typedef UINT16 EFI_IMAGE_ID;
typedef UINT16 EFI_QUESTION_ID;
diff --git a/MdeModulePkg/Core/Dxe/Event/Event.h b/MdeModulePkg/Core/Dxe/Event/Event.h
index 8141c5003eec..42590cb1dd09 100644
--- a/MdeModulePkg/Core/Dxe/Event/Event.h
+++ b/MdeModulePkg/Core/Dxe/Event/Event.h
@@ -37,7 +37,7 @@ typedef struct {
} TIMER_EVENT_INFO;

#define EVENT_SIGNATURE SIGNATURE_32('e','v','n','t')
-typedef struct {
+typedef struct EFI_EVENT_OBJECT {
UINTN Signature;
UINT32 Type;
UINT32 SignalCount;
diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.h b/MdeModulePkg/Core/Dxe/Hand/Handle.h
index 83eb2b9f3afe..1f1ab3274e8a 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.h
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.h
@@ -15,7 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 0908e7f4e9e7..c55da58d465e 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -145,7 +145,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
index 50d4c96edb63..bfebbb1f8182 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h
@@ -88,7 +88,7 @@ struct _EFI_AML_NODE_LIST {
// This buffer should not be freed.
// Size is the total size of this ACPI node buffer.
//
-typedef struct {
+typedef struct EFI_ACPI_OBJECT {
UINT32 Signature;
UINT8 *Buffer;
UINTN Size;
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
index 4a3feab94df5..48972d0fcad6 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h
@@ -274,7 +274,7 @@ typedef struct _HII_DATABASE_PACKAGE_LIST_INSTANCE {

#define HII_HANDLE_SIGNATURE SIGNATURE_32 ('h','i','h','l')

-typedef struct {
+typedef struct EFI_HII_OBJECT {
UINTN Signature;
LIST_ENTRY Handle;
UINTN Key;
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 4d0eed273f50..dcf91bc5e916 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -105,7 +105,7 @@ typedef struct {
///
/// IHANDLE - contains a list of protocol handles
///
-typedef struct {
+typedef struct EFI_OBJECT {
UINTN Signature;
/// All handles list of IHANDLE
LIST_ENTRY AllHandles;
--
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47388): https://edk2.groups.io/g/devel/message/47388
Mute This Topic: https://groups.io/mt/34180199/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@...]
-=-=-=-=-=-=

Re: [PATCH 05/35] EmulatorPkg/DxeTimerLib: drop superfluous cast

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Andrew Fish <afish@...>; Justen, Jordan L <jordan.l.justen@...>; Ni, Ray <ray.ni@...>
Subject: [PATCH 05/35] EmulatorPkg/DxeTimerLib: drop superfluous cast

"gTimerEvent" has type EFI_EVENT already, drop the superfluous cast.

Cc: Andrew Fish <afish@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
index 14cae4214c66..6fb5d8f3aaea 100644
--- a/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
+++ b/EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c
@@ -40,7 +40,7 @@ RegisterTimerArchProtocol (
gTimerPeriod = MultU64x32 (gTimerPeriod, 100);

if (gTimerEvent == NULL) {
- Status = gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, (VOID **)&gTimerEvent);
+ Status = gBS->CreateEvent (EVT_TIMER, 0, NULL, NULL, &gTimerEvent);
ASSERT_EFI_ERROR (Status);
}
}
--
2.19.1.3.g30247aa5d201

Re: [PATCH 06/35] EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration

Ni, Ray
 

Reviewed-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Laszlo Ersek <lersek@...>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Andrew Fish <afish@...>; Justen, Jordan L <jordan.l.justen@...>; Ni, Ray <ray.ni@...>
Subject: [PATCH 06/35] EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration

EFI_REGISTER_KEYSTROKE_NOTIFY and EFI_UNREGISTER_KEYSTROKE_NOTIFY require
the notification handle to have type (VOID*). The notification handle has
nothing to do with the EFI_HANDLE type.

This change is a semantic fix; functionally, it's a no-op.

Cc: Andrew Fish <afish@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

EmulatorPkg/EmuGopDxe/GopInput.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/EmulatorPkg/EmuGopDxe/GopInput.c b/EmulatorPkg/EmuGopDxe/GopInput.c
index fdd0b4911555..2a23564a2173 100644
--- a/EmulatorPkg/EmuGopDxe/GopInput.c
+++ b/EmulatorPkg/EmuGopDxe/GopInput.c
@@ -517,7 +517,7 @@ EmuGopSimpleTextInExRegisterKeyNotify (
IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *This,
IN EFI_KEY_DATA *KeyData,
IN EFI_KEY_NOTIFY_FUNCTION KeyNotificationFunction,
- OUT EFI_HANDLE *NotifyHandle
+ OUT VOID **NotifyHandle
)
{
EFI_STATUS Status;
@@ -600,7 +600,7 @@ EFI_STATUS
EFIAPI
EmuGopSimpleTextInExUnregisterKeyNotify (
IN EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL *This,
- IN EFI_HANDLE NotificationHandle
+ IN VOID *NotificationHandle
)
/*++

--
2.19.1.3.g30247aa5d201

[PATCH 35/35] UefiPayloadPkg/BlSupportDxe: fix ReserveResourceInGcd() calls

Laszlo Ersek
 

The last parameter of ReserveResourceInGcd() is "ImageHandle", forwarded
in turn to gDS->AllocateMemorySpace() or gDS->AllocateIoSpace() as "owner=
"
image handle.

But BlDxeEntryPoint() passes "SystemTable" as "ImageHandle".

Compilers have not flagged it because EFI_HANDLE (the type of
"ImageHandle") is unfortunately specified as (VOID*), and
(EFI_SYSTEM_TABLE*) converts to (VOID*) silently.

Hand the entry point function's "ImageHandle" parameter to
ReserveResourceInGcd(). This fixes an actual bug.

Cc: Benjamin You <benjamin.you@...>
Cc: Guo Dong <guo.dong@...>
Cc: Maurice Ma <maurice.ma@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c b/UefiPayloadPkg/=
BlSupportDxe/BlSupportDxe.c
index bcee4cd9bc41..28dfc8fc5545 100644
--- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
+++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
@@ -106,10 +106,10 @@ BlDxeEntryPoint (
//
// Report MMIO/IO Resources
//
- Status =3D ReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo,=
0xFEC00000, SIZE_4KB, 0, SystemTable); // IOAPIC
+ Status =3D ReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo,=
0xFEC00000, SIZE_4KB, 0, ImageHandle); // IOAPIC
ASSERT_EFI_ERROR (Status);
=20
- Status =3D ReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo,=
0xFED00000, SIZE_1KB, 0, SystemTable); // HPET
+ Status =3D ReserveResourceInGcd (TRUE, EfiGcdMemoryTypeMemoryMappedIo,=
0xFED00000, SIZE_1KB, 0, ImageHandle); // HPET
ASSERT_EFI_ERROR (Status);
=20
//
--=20
2.19.1.3.g30247aa5d201

[PATCH 34/35] UefiPayloadPkg/BlSupportPei: fix MMCONFIG assignment from XSDT

Laszlo Ersek
 

(This patch is unrelated to the rest of this series; its purpose is to
enable building the UefiPayloadPkg DSC files with GCC.)

When building "UefiPayloadPkg/UefiPayloadPkgIa32.dsc" with GCC48 for the
DEBUG target, the compiler reports that "Entry32" may be used
uninitialized in ParseAcpiInfo(), in the XSDT branch.

Code inspection proves the compiler right. In the XSDT branch, the code
from the RSDT branch must have been duplicated, and "Entry32" references
were replaced with "Entry64" -- except where "MmCfgHdr" is assigned.

Fix this bug by introducing a helper variable called "Signature", so that
we have to refer to "Entry32" or "Entry64" only once per loop body.

Cc: Benjamin You <benjamin.you@...>
Cc: Guo Dong <guo.dong@...>
Cc: Maurice Ma <maurice.ma@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

UefiPayloadPkg/BlSupportPei/BlSupportPei.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c b/UefiPayloadPkg/=
BlSupportPei/BlSupportPei.c
index 90433b609f22..22972453117a 100644
--- a/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
+++ b/UefiPayloadPkg/BlSupportPei/BlSupportPei.c
@@ -164,6 +164,7 @@ ParseAcpiInfo (
UINT64 *Entry64;
UINTN Entry64Num;
UINTN Idx;
+ UINT32 *Signature;
EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *MmCfgH=
dr;
EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOC=
ATION_STRUCTURE *MmCfgBase;
=20
@@ -181,13 +182,14 @@ ParseAcpiInfo (
Entry32 =3D (UINT32 *)(Rsdt + 1);
Entry32Num =3D (Rsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) =
2;
for (Idx =3D 0; Idx < Entry32Num; Idx++) {
- if (*(UINT32 *)(UINTN)(Entry32[Idx]) =3D=3D EFI_ACPI_3_0_FIXED_ACP=
I_DESCRIPTION_TABLE_SIGNATURE) {
- Fadt =3D (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)(En=
try32[Idx]);
+ Signature =3D (UINT32 *)(UINTN)Entry32[Idx];
+ if (*Signature =3D=3D EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SI=
GNATURE) {
+ Fadt =3D (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
DEBUG ((DEBUG_INFO, "Found Fadt in Rsdt\n"));
}
=20
- if (*(UINT32 *)(UINTN)(Entry32[Idx]) =3D=3D EFI_ACPI_5_0_PCI_EXPRE=
SS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNA=
TURE) {
- MmCfgHdr =3D (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_=
TABLE_HEADER *)(UINTN)(Entry32[Idx]);
+ if (*Signature =3D=3D EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFI=
GURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
+ MmCfgHdr =3D (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_=
TABLE_HEADER *)Signature;
DEBUG ((DEBUG_INFO, "Found MM config address in Rsdt\n"));
}
=20
@@ -205,13 +207,14 @@ ParseAcpiInfo (
Entry64 =3D (UINT64 *)(Xsdt + 1);
Entry64Num =3D (Xsdt->Length - sizeof(EFI_ACPI_DESCRIPTION_HEADER)) =
3;
for (Idx =3D 0; Idx < Entry64Num; Idx++) {
- if (*(UINT32 *)(UINTN)(Entry64[Idx]) =3D=3D EFI_ACPI_3_0_FIXED_ACP=
I_DESCRIPTION_TABLE_SIGNATURE) {
- Fadt =3D (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)(UINTN)(En=
try64[Idx]);
+ Signature =3D (UINT32 *)(UINTN)Entry64[Idx];
+ if (*Signature =3D=3D EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE_SI=
GNATURE) {
+ Fadt =3D (EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *)Signature;
DEBUG ((DEBUG_INFO, "Found Fadt in Xsdt\n"));
}
=20
- if (*(UINT32 *)(UINTN)(Entry64[Idx]) =3D=3D EFI_ACPI_5_0_PCI_EXPRE=
SS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNA=
TURE) {
- MmCfgHdr =3D (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_=
TABLE_HEADER *)(UINTN)(Entry32[Idx]);
+ if (*Signature =3D=3D EFI_ACPI_5_0_PCI_EXPRESS_MEMORY_MAPPED_CONFI=
GURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE) {
+ MmCfgHdr =3D (EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_=
TABLE_HEADER *)Signature;
DEBUG ((DEBUG_INFO, "Found MM config address in Xsdt\n"));
}
=20
--=20
2.19.1.3.g30247aa5d201

[PATCH 33/35] StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader tracking

Laszlo Ersek
 

The FvHasBeenProcessed() and FvIsBeingProcesssed() functions make sure
that every firmware volume is processed only once (every driver in every
firmware volume should be discovered only once). For this, the functions
use a linked list.

In MdeModulePkg's DXE Core and SMM Core, the key used for identifying
those firmware volumes that have been processed is the EFI_HANDLE on whic=
h
the DXE or SMM firmware volume protocol is installed. In the
StandaloneMmPkg core however, the key is the address of the firmware
volume header; that is, it has type (EFI_FIRMWARE_VOLUME_HEADER*).

(EFI_FIRMWARE_VOLUME_HEADER*) has nothing to do with EFI_HANDLE.
EFI_HANDLE just happens to be specified as (VOID*), and therefore the
conversion between (EFI_FIRMWARE_VOLUME_HEADER*) and EFI_HANDLE is silent=
.

(The FvHasBeenProcessed() and FvIsBeingProcesssed() functions were likely
copied verbatim from MdeModulePkg's DXE Core and/or the SMM Core, and not
flagged by the compiler in StandaloneMmPkg due to UEFI regrettably
specifying EFI_HANDLE as (VOID*), thereby enabling the above implicit
conversion.)

We should not exploit this circumstance. Represent the key type faithfull=
y
instead.

This is a semantic fix; there is no change in operation.

Cc: Achin Gupta <achin.gupta@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
StandaloneMmPkg/Core/Dispatcher.c | 80 +++++++++++---------
StandaloneMmPkg/Core/FwVol.c | 16 ++--
3 files changed, 52 insertions(+), 46 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Co=
re/StandaloneMmCore.h
index dcf91bc5e916..23ddbe169faf 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -67,7 +67,7 @@ typedef struct {
=20
LIST_ENTRY ScheduledLink; // mScheduledQueue
=20
- EFI_HANDLE FvHandle;
+ EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
EFI_GUID FileName;
VOID *Pe32Data;
UINTN Pe32DataSize;
diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dis=
patcher.c
index 3788389f95ed..9853445a64a1 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -5,7 +5,7 @@
is added to the mDiscoveredList. The Before, and After Depex=
are
pre-processed as drivers are added to the mDiscoveredList. I=
f an Apriori
file exists in the FV those drivers are addeded to the
- mScheduledQueue. The mFvHandleList is used to make sure a
+ mScheduledQueue. The mFwVolList is used to make sure a
FV is only processed once.
=20
Step #2 - Dispatch. Remove driver from the mScheduledQueue and load an=
d
@@ -40,13 +40,13 @@
//
// MM Dispatcher Data structures
//
-#define KNOWN_HANDLE_SIGNATURE SIGNATURE_32('k','n','o','w')
+#define KNOWN_FWVOL_SIGNATURE SIGNATURE_32('k','n','o','w')
=20
typedef struct {
- UINTN Signature;
- LIST_ENTRY Link; // mFvHandleList
- EFI_HANDLE Handle;
-} KNOWN_HANDLE;
+ UINTN Signature;
+ LIST_ENTRY Link; // mFwVolList
+ EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
+} KNOWN_FWVOL;
=20
//
// Function Prototypes
@@ -86,9 +86,10 @@ LIST_ENTRY mDiscoveredList =3D INITIALIZE_LIST_HEAD_V=
ARIABLE (mDiscoveredList);
LIST_ENTRY mScheduledQueue =3D INITIALIZE_LIST_HEAD_VARIABLE (mSchedule=
dQueue);
=20
//
-// List of handles who's Fv's have been parsed and added to the mFwDrive=
rList.
+// List of firmware volume headers whose containing firmware volumes hav=
e been
+// parsed and added to the mFwDriverList.
//
-LIST_ENTRY mFvHandleList =3D INITIALIZE_LIST_HEAD_VARIABLE (mFvHandleLi=
st);
+LIST_ENTRY mFwVolList =3D INITIALIZE_LIST_HEAD_VARIABLE (mFwVolList);
=20
//
// Flag for the MM Dispacher. TRUE if dispatcher is execuing.
@@ -769,26 +770,30 @@ MmInsertOnScheduledQueueWhileProcessingBeforeAndAft=
er (
}
=20
/**
- Return TRUE if the Fv has been processed, FALSE if not.
+ Return TRUE if the firmware volume has been processed, FALSE if not.
=20
- @param FvHandle The handle of a FV that's being tested
+ @param FwVolHeader The header of the firmware volume that's=
being
+ tested.
=20
- @retval TRUE Fv protocol on FvHandle has been process=
ed
- @retval FALSE Fv protocol on FvHandle has not yet been
- processed
+ @retval TRUE The firmware volume denoted by FwVolHead=
er has
+ been processed
+ @retval FALSE The firmware volume denoted by FwVolHead=
er has
+ not yet been processed
=20
**/
BOOLEAN
FvHasBeenProcessed (
- IN EFI_HANDLE FvHandle
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
)
{
LIST_ENTRY *Link;
- KNOWN_HANDLE *KnownHandle;
+ KNOWN_FWVOL *KnownFwVol;
=20
- for (Link =3D mFvHandleList.ForwardLink; Link !=3D &mFvHandleList; Lin=
k =3D Link->ForwardLink) {
- KnownHandle =3D CR (Link, KNOWN_HANDLE, Link, KNOWN_HANDLE_SIGNATURE=
);
- if (KnownHandle->Handle =3D=3D FvHandle) {
+ for (Link =3D mFwVolList.ForwardLink;
+ Link !=3D &mFwVolList;
+ Link =3D Link->ForwardLink) {
+ KnownFwVol =3D CR (Link, KNOWN_FWVOL, Link, KNOWN_FWVOL_SIGNATURE);
+ if (KnownFwVol->FwVolHeader =3D=3D FwVolHeader) {
return TRUE;
}
}
@@ -796,28 +801,29 @@ FvHasBeenProcessed (
}
=20
/**
- Remember that Fv protocol on FvHandle has had it's drivers placed on t=
he
- mDiscoveredList. This fucntion adds entries on the mFvHandleList. Item=
s are
- never removed/freed from the mFvHandleList.
+ Remember that the firmware volume denoted by FwVolHeader has had its d=
rivers
+ placed on mDiscoveredList. This function adds entries to mFwVolList. I=
tems
+ are never removed/freed from mFwVolList.
=20
- @param FvHandle The handle of a FV that has been process=
ed
+ @param FwVolHeader The header of the firmware volume that's=
being
+ processed.
=20
**/
VOID
FvIsBeingProcesssed (
- IN EFI_HANDLE FvHandle
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
)
{
- KNOWN_HANDLE *KnownHandle;
+ KNOWN_FWVOL *KnownFwVol;
=20
- DEBUG ((DEBUG_INFO, "FvIsBeingProcesssed - 0x%08x\n", FvHandle));
+ DEBUG ((DEBUG_INFO, "FvIsBeingProcesssed - 0x%08x\n", KnownFwVol));
=20
- KnownHandle =3D AllocatePool (sizeof (KNOWN_HANDLE));
- ASSERT (KnownHandle !=3D NULL);
+ KnownFwVol =3D AllocatePool (sizeof (KNOWN_FWVOL));
+ ASSERT (KnownFwVol !=3D NULL);
=20
- KnownHandle->Signature =3D KNOWN_HANDLE_SIGNATURE;
- KnownHandle->Handle =3D FvHandle;
- InsertTailList (&mFvHandleList, &KnownHandle->Link);
+ KnownFwVol->Signature =3D KNOWN_FWVOL_SIGNATURE;
+ KnownFwVol->FwVolHeader =3D FwVolHeader;
+ InsertTailList (&mFwVolList, &KnownFwVol->Link);
}
=20
/**
@@ -842,12 +848,12 @@ FvIsBeingProcesssed (
**/
EFI_STATUS
MmAddToDriverList (
- IN EFI_HANDLE FvHandle,
- IN VOID *Pe32Data,
- IN UINTN Pe32DataSize,
- IN VOID *Depex,
- IN UINTN DepexSize,
- IN EFI_GUID *DriverName
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader,
+ IN VOID *Pe32Data,
+ IN UINTN Pe32DataSize,
+ IN VOID *Depex,
+ IN UINTN DepexSize,
+ IN EFI_GUID *DriverName
)
{
EFI_MM_DRIVER_ENTRY *DriverEntry;
@@ -863,7 +869,7 @@ MmAddToDriverList (
=20
DriverEntry->Signature =3D EFI_MM_DRIVER_ENTRY_SIGNATURE;
CopyGuid (&DriverEntry->FileName, DriverName);
- DriverEntry->FvHandle =3D FvHandle;
+ DriverEntry->FwVolHeader =3D FwVolHeader;
DriverEntry->Pe32Data =3D Pe32Data;
DriverEntry->Pe32DataSize =3D Pe32DataSize;
DriverEntry->Depex =3D Depex;
diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 9fe0c257a43a..99ecf4af4714 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -24,22 +24,22 @@ EFI_FV_FILETYPE mMmFileTypes[] =3D {
=20
EFI_STATUS
MmAddToDriverList (
- IN EFI_HANDLE FvHandle,
- IN VOID *Pe32Data,
- IN UINTN Pe32DataSize,
- IN VOID *Depex,
- IN UINTN DepexSize,
- IN EFI_GUID *DriverName
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader,
+ IN VOID *Pe32Data,
+ IN UINTN Pe32DataSize,
+ IN VOID *Depex,
+ IN UINTN DepexSize,
+ IN EFI_GUID *DriverName
);
=20
BOOLEAN
FvHasBeenProcessed (
- IN EFI_HANDLE FvHandle
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
);
=20
VOID
FvIsBeingProcesssed (
- IN EFI_HANDLE FvHandle
+ IN EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader
);
=20
EFI_STATUS
--=20
2.19.1.3.g30247aa5d201

[PATCH 32/35] ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug

Laszlo Ersek
 

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@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

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/Libr=
ary/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 !=3D NULL) {
//
// Call EFI Shell version.
- // Due to oddity in the EFI shell we want to dereference the ParentH=
andle here
//
- CmdStatus =3D (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 fir=
st
+ // parameter, with type (EFI_HANDLE*).
+ // 3. In the EdkShell implementation, SEnvExecute() implements the
+ // Execute() member function. It passes "ParentImageHandle" corre=
ctly to
+ // SEnvDoExecute().
+ // 4. SEnvDoExecute() takes the (EFI_HANDLE*), and passes it directl=
y --
+ // 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 =3D (mEfiShellEnvironment2->Execute((EFI_HANDLE *)*ParentH=
andle,
CommandLine,
Output));
//
--=20
2.19.1.3.g30247aa5d201

[PATCH 31/35] ShellPkg/UefiShellDebug1CommandsLib: fix ShellCloseFile() call

Laszlo Ersek
 

In the FileBufferSave() function, we invoke ShellCloseFile() if "Director=
y
Can Not Be Saved".

The ShellCloseFile() function takes a (SHELL_FILE_HANDLE*) parameter
called "FileHandle", and correctly passes the de-referenced (*FileHandle)
to EFI_SHELL_CLOSE_FILE, which takes a SHELL_FILE_HANDLE.

However, FileBufferSave() passes SHELL_FILE_HANDLE to ShellCloseFile(),
not the expected (SHELL_FILE_HANDLE*). Correct it.

This fixes an actual bug that has remained hidden for two reasons:

- pointer-to-VOID converts from/to any pointer-to-object type silently,
- the bug is on an error path which has likely never fired in practice.

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

Notes:
tested: edit (saving a file)

ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.=
c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c
index 464f9de38e52..fd324cc4a861 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c
@@ -1462,7 +1462,7 @@ FileBufferSave (
=20
if (Info !=3D NULL && Info->Attribute & EFI_FILE_DIRECTORY) {
StatusBarSetStatusString (L"Directory Can Not Be Saved");
- ShellCloseFile(FileHandle);
+ ShellCloseFile (&FileHandle);
FreePool(Info);
return EFI_LOAD_ERROR;
}
--=20
2.19.1.3.g30247aa5d201

[PATCH 30/35] ShellPkg: stop taking EFI_HANDLE in place of SHELL_FILE_HANDLE

Laszlo Ersek
 

The TouchFileByHandle() and IsDirectoryEmpty() functions are passed
SHELL_FILE_HANDLE parameters, and they use those parameters correctly.
However, their parameter lists say EFI_HANDLE.

Spell out the right type in the parameter lists.

In practice, this change is a no-op (because, quite regrettably, both
EFI_HANDLE and SHELL_FILE_HANDLE are specified to be typedefs of (VOID*))=
.

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

Notes:
tested: rm, touch

ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c | 2 +-
ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c b/ShellPkg/=
Library/UefiShellLevel2CommandsLib/Rm.c
index 3a1196f1529e..59f7eec376f2 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
@@ -24,7 +24,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] =3D {
**/
BOOLEAN
IsDirectoryEmpty (
- IN EFI_HANDLE FileHandle
+ IN SHELL_FILE_HANDLE FileHandle
)
{
EFI_STATUS Status;
diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c b/ShellP=
kg/Library/UefiShellLevel3CommandsLib/Touch.c
index 0f00344c815e..a215f5774c69 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
@@ -21,7 +21,7 @@
**/
EFI_STATUS
TouchFileByHandle (
- IN EFI_HANDLE Handle
+ IN SHELL_FILE_HANDLE Handle
)
{
EFI_STATUS Status;
--=20
2.19.1.3.g30247aa5d201

[PATCH 29/35] ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE

Laszlo Ersek
 

The UefiShell*CommandsLib instances have constructor functions that do
something like:

gHiiHandle =3D 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 <jaben.carsey@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

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/UefiShellNetwork1CommandsL=
ib.h | 2 +-
ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL=
ib.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/UefiShellNetwork1CommandsL=
ib.c | 2 +-
ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsL=
ib.c | 2 +-
25 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/ShellPkg/Include/Library/ShellCommandLib.h b/ShellPkg/Includ=
e/Library/ShellCommandLib.h
index 287bc0eba7f9..63fcac82a2de 100644
--- a/ShellPkg/Include/Library/ShellCommandLib.h
+++ b/ShellPkg/Include/Library/ShellCommandLib.h
@@ -136,7 +136,7 @@ ShellCommandRegisterCommandName (
IN UINT32 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
);
=20
diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Libra=
ry/ShellLib.h
index 31594796cd21..1dc41f2cc11b 100644
--- a/ShellPkg/Include/Library/ShellLib.h
+++ b/ShellPkg/Include/Library/ShellLib.h
@@ -965,7 +965,7 @@ ShellPrintHiiEx(
IN INT32 Row OPTIONAL,
IN CONST CHAR8 *Language OPTIONAL,
IN CONST EFI_STRING_ID HiiFormatStringId,
- IN CONST EFI_HANDLE HiiFormatHandle,
+ IN CONST EFI_HII_HANDLE HiiFormatHandle,
...
);
=20
@@ -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
);
=20
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h b/ShellPkg/Dyn=
amicCommand/DpDynamicCommand/Dp.h
index 43aa4505ee37..e446cccde923 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h
@@ -36,7 +36,7 @@
#include <Library/UefiHiiServicesLib.h>
#include <Library/PerformanceLib.h>
=20
-extern EFI_HANDLE mDpHiiHandle;
+extern EFI_HII_HANDLE mDpHiiHandle;
=20
#define DP_MAJOR_VERSION 2
#define DP_MINOR_VERSION 5
@@ -133,7 +133,7 @@ RunDp (
=20
@return HII handle.
**/
-EFI_HANDLE
+EFI_HII_HANDLE
InitializeHiiPackage (
EFI_HANDLE ImageHandle
);
diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h b/ShellPkg=
/DynamicCommand/TftpDynamicCommand/Tftp.h
index 7a9ed4724e1f..4cd778436813 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h
@@ -30,7 +30,7 @@
#include <Library/PrintLib.h>
#include <Library/UefiHiiServicesLib.h>
=20
-extern EFI_HANDLE mTftpHiiHandle;
+extern EFI_HII_HANDLE mTftpHiiHandle;
=20
typedef struct {
UINTN FileSize;
@@ -62,7 +62,7 @@ RunTftp (
=20
@return HII handle.
**/
-EFI_HANDLE
+EFI_HII_HANDLE
InitializeHiiPackage (
EFI_HANDLE ImageHandle
);
diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h b=
/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h
index 36fe628a8c68..8ecc2f6bf5a2 100644
--- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h
+++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h
@@ -46,7 +46,7 @@ typedef struct{
SHELL_GET_MAN_FILENAME GetManFileName;
SHELL_RUN_COMMAND CommandHandler;
BOOLEAN LastError;
- EFI_HANDLE HiiHandle;
+ EFI_HII_HANDLE HiiHandle;
EFI_STRING_ID ManFormatHelp;
} SHELL_COMMAND_INTERNAL_LIST_ENTRY;
=20
diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1C=
ommandsLib.h b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug=
1CommandsLib.h
index 32a933b9f062..082d488cb283 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands=
Lib.h
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands=
Lib.h
@@ -52,7 +52,7 @@
#include <Library/HandleParsingLib.h>
=20
=20
-extern EFI_HANDLE gShellDebug1HiiHandle;
+extern EFI_HII_HANDLE gShellDebug1HiiHandle;
=20
/**
Function returns a system configuration table that is stored in the
diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver=
1CommandsLib.h b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDr=
iver1CommandsLib.h
index 7e0b8b094057..ee795c4ce024 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Comman=
dsLib.h
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Comman=
dsLib.h
@@ -58,7 +58,7 @@
#include <Library/HandleParsingLib.h>
=20
=20
-extern EFI_HANDLE gShellDriver1HiiHandle;
+extern EFI_HII_HANDLE gShellDriver1HiiHandle;
extern BOOLEAN gInReconnect;
=20
/**
diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1C=
ommandsLib.h b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel=
1CommandsLib.h
index 55acdd2b1f95..f2f9cc5dcf3b 100644
--- a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Commands=
Lib.h
+++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Commands=
Lib.h
@@ -33,7 +33,7 @@
#include <Library/HiiLib.h>
#include <Library/FileHandleLib.h>
=20
-extern EFI_HANDLE gShellLevel1HiiHandle;
+extern EFI_HII_HANDLE gShellLevel1HiiHandle;
=20
/**
Function for 'stall' command.
diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2C=
ommandsLib.h b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel=
2CommandsLib.h
index 6d522d4bb4a1..77be6f1a12c7 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands=
Lib.h
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands=
Lib.h
@@ -43,7 +43,7 @@
#include <Library/FileHandleLib.h>
=20
extern CONST CHAR16 mFileName[];
-extern EFI_HANDLE gShellLevel2HiiHandle;
+extern EFI_HII_HANDLE gShellLevel2HiiHandle;
=20
/**
Function for 'attrib' command.
diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3C=
ommandsLib.h b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel=
3CommandsLib.h
index 2d97ae4d3c91..c095b9275ed0 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Commands=
Lib.h
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Commands=
Lib.h
@@ -32,7 +32,7 @@
#include <Library/HiiLib.h>
#include <Library/FileHandleLib.h>
=20
-extern EFI_HANDLE gShellLevel3HiiHandle;
+extern EFI_HII_HANDLE gShellLevel3HiiHandle;
=20
/**
Function for 'type' command.
diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwo=
rk1CommandsLib.h b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShel=
lNetwork1CommandsLib.h
index d4ed8c04652d..fddada2efa48 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm=
andsLib.h
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm=
andsLib.h
@@ -38,7 +38,7 @@
#include <Library/DevicePathLib.h>
#include <Library/PrintLib.h>
=20
-extern EFI_HANDLE gShellNetwork1HiiHandle;
+extern EFI_HII_HANDLE gShellNetwork1HiiHandle;
=20
/**
Function for 'ping' command.
diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwo=
rk2CommandsLib.h b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShel=
lNetwork2CommandsLib.h
index 9a5db32f2b76..9ea42cf26d53 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm=
andsLib.h
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm=
andsLib.h
@@ -27,7 +27,7 @@
#include <Library/HiiLib.h>
#include <Library/NetLib.h>
=20
-extern EFI_HANDLE gShellNetwork2HiiHandle;
+extern EFI_HII_HANDLE gShellNetwork2HiiHandle;
=20
/**
Function for 'ping6' command.
diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c b/ShellPkg/Dyn=
amicCommand/DpDynamicCommand/Dp.c
index 735cdcbcc018..4ec4c18348bd 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -36,7 +36,7 @@ typedef struct {
=20
#pragma pack()
=20
-EFI_HANDLE mDpHiiHandle;
+EFI_HII_HANDLE mDpHiiHandle;
=20
typedef struct {
EFI_HANDLE Handle;
@@ -924,14 +924,14 @@ Done:
=20
@return HII handle.
**/
-EFI_HANDLE
+EFI_HII_HANDLE
InitializeHiiPackage (
EFI_HANDLE ImageHandle
)
{
EFI_STATUS Status;
EFI_HII_PACKAGE_LIST_HEADER *PackageList;
- EFI_HANDLE HiiHandle;
+ EFI_HII_HANDLE HiiHandle;
=20
//
// Retrieve HII package list from ImageHandle
diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c b/ShellPkg=
/DynamicCommand/TftpDynamicCommand/Tftp.c
index 607899032e9d..f28da9af723c 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
@@ -11,7 +11,7 @@
#include "Tftp.h"
=20
#define IP4_CONFIG2_INTERFACE_INFO_NAME_LENGTH 32
-EFI_HANDLE mTftpHiiHandle;
+EFI_HII_HANDLE mTftpHiiHandle;
=20
/*
Constant strings and definitions related to the message indicating th=
e amount of
@@ -1087,14 +1087,14 @@ CheckPacket (
=20
@return HII handle.
**/
-EFI_HANDLE
+EFI_HII_HANDLE
InitializeHiiPackage (
EFI_HANDLE ImageHandle
)
{
EFI_STATUS Status;
EFI_HII_PACKAGE_LIST_HEADER *PackageList;
- EFI_HANDLE HiiHandle;
+ EFI_HII_HANDLE HiiHandle;
=20
//
// Retrieve HII package list from ImageHandle
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c=
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index f179c4109223..f62d30ef677a 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -14,7 +14,7 @@
#include <PiDxe.h>
#include <Protocol/FirmwareVolume2.h>
=20
-EFI_HANDLE mHandleParsingHiiHandle =3D NULL;
+EFI_HII_HANDLE mHandleParsingHiiHandle =3D NULL;
HANDLE_INDEX_LIST mHandleList =3D {{{NULL,NULL},0,0},0};
GUID_INFO_BLOCK *mGuidList;
UINTN mGuidListCount;
diff --git a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgComman=
dLib.c b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib=
.c
index e8b48b4990dd..f8bcaebe46c8 100644
--- a/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
+++ b/ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
@@ -38,7 +38,7 @@
#include <Library/UefiBootManagerLib.h>
=20
STATIC CONST CHAR16 mFileName[] =3D L"ShellCommands";
-STATIC EFI_HANDLE gShellBcfgHiiHandle =3D NULL;
+STATIC EFI_HII_HANDLE gShellBcfgHiiHandle =3D NULL;
=20
typedef enum {
BcfgTargetBootOrder =3D 0,
diff --git a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c b=
/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
index 826ced30a8c8..4c48b65fbc1d 100644
--- a/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
+++ b/ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
@@ -554,7 +554,7 @@ ShellCommandRegisterCommandName (
IN UINT32 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/Library/UefiShellDebug1CommandsLib/UefiShellDebug1C=
ommandsLib.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug=
1CommandsLib.c
index ddce3bef5a30..f918867f47af 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands=
Lib.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands=
Lib.c
@@ -10,7 +10,7 @@
#include <Library/BcfgCommandLib.h>
=20
STATIC CONST CHAR16 mFileName[] =3D L"Debug1Commands";
-EFI_HANDLE gShellDebug1HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellDebug1HiiHandle =3D NULL;
=20
/**
Gets the debug file name. This will be used if HII is not working.
diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver=
1CommandsLib.c b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDr=
iver1CommandsLib.c
index 4a05fa9942c4..e2219c62ec25 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Comman=
dsLib.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Comman=
dsLib.c
@@ -9,7 +9,7 @@
#include "UefiShellDriver1CommandsLib.h"
=20
STATIC CONST CHAR16 mFileName[] =3D L"Driver1Commands";
-EFI_HANDLE gShellDriver1HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellDriver1HiiHandle =3D NULL;
BOOLEAN gInReconnect =3D FALSE;
=20
/**
diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1C=
ommandsLib.c b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel=
1CommandsLib.c
index ecbee99e3b3d..88cddd88ddc4 100644
--- a/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Commands=
Lib.c
+++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Commands=
Lib.c
@@ -10,7 +10,7 @@
#include "UefiShellLevel1CommandsLib.h"
=20
STATIC CONST CHAR16 mFileName[] =3D L"ShellCommands";
-EFI_HANDLE gShellLevel1HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellLevel1HiiHandle =3D NULL;
=20
/**
Return the help text filename. Only used if no HII information found.
diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2C=
ommandsLib.c b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel=
2CommandsLib.c
index c2a0bb492fbb..69427637bb87 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands=
Lib.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands=
Lib.c
@@ -29,7 +29,7 @@
#include "UefiShellLevel2CommandsLib.h"
=20
CONST CHAR16 mFileName[] =3D L"ShellCommands";
-EFI_HANDLE gShellLevel2HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellLevel2HiiHandle =3D NULL;
=20
/**
Get the filename to get help text from if not using HII.
diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3C=
ommandsLib.c b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel=
3CommandsLib.c
index 7d2cc4a48371..ce4afd117aa1 100644
--- a/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Commands=
Lib.c
+++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Commands=
Lib.c
@@ -9,7 +9,7 @@
#include "UefiShellLevel3CommandsLib.h"
=20
CONST CHAR16 gShellLevel3FileName[] =3D L"ShellCommands";
-EFI_HANDLE gShellLevel3HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellLevel3HiiHandle =3D NULL;
=20
/**
return the filename to get help from is not using HII.
diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Libr=
ary/UefiShellLib/UefiShellLib.c
index 5be530092e1b..835d0f88ca74 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -2997,7 +2997,7 @@ ShellPrintHiiEx(
IN INT32 Row OPTIONAL,
IN CONST CHAR8 *Language OPTIONAL,
IN CONST EFI_STRING_ID HiiFormatStringId,
- IN CONST EFI_HANDLE HiiFormatHandle,
+ IN CONST EFI_HII_HANDLE HiiFormatHandle,
...
)
{
@@ -3609,7 +3609,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/Library/UefiShellNetwork1CommandsLib/UefiShellNetwo=
rk1CommandsLib.c b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShel=
lNetwork1CommandsLib.c
index 7e823cabd2fe..9a2b23fdc5ba 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm=
andsLib.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm=
andsLib.c
@@ -8,7 +8,7 @@
#include "UefiShellNetwork1CommandsLib.h"
=20
CONST CHAR16 gShellNetwork1FileName[] =3D L"ShellCommands";
-EFI_HANDLE gShellNetwork1HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellNetwork1HiiHandle =3D NULL;
=20
/**
return the file name of the help text file if not using HII.
diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwo=
rk2CommandsLib.c b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShel=
lNetwork2CommandsLib.c
index 5a7ffbfa19eb..4aab4295c1ba 100644
--- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm=
andsLib.c
+++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm=
andsLib.c
@@ -8,7 +8,7 @@
#include "UefiShellNetwork2CommandsLib.h"
=20
CONST CHAR16 gShellNetwork2FileName[] =3D L"ShellCommands";
-EFI_HANDLE gShellNetwork2HiiHandle =3D NULL;
+EFI_HII_HANDLE gShellNetwork2HiiHandle =3D NULL;
=20
/**
return the file name of the help text file if not using HII.
--=20
2.19.1.3.g30247aa5d201

[PATCH 28/35] ShellPkg/UefiShellDriver1CommandsLib: fix parameter list typo

Laszlo Ersek
 

The ShellCommandRunConnect() function passes EFI_HANDLE -- (VOID*) --
objects to ConvertAndConnectControllers(), and
ConvertAndConnectControllers() passes those to gBS->OpenProtocol().

Accordingly, ConvertAndConnectControllers() should specify EFI_HANDLE
parameter types, not (EFI_HANDLE*) -- (VOID**) -- types.

This typo is masked because (VOID*) converts to and from any
pointer-to-object type silently.

Note that functionally speaking there is no problem, so this patch does
not change beavior, only cleans up the code.

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

Notes:
tested with "connect -r"

ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c b/She=
llPkg/Library/UefiShellDriver1CommandsLib/Connect.c
index 359394dfd291..3f4e132674ea 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
@@ -346,8 +346,8 @@ ShellConnectFromDevPaths (
**/
EFI_STATUS
ConvertAndConnectControllers (
- IN EFI_HANDLE *Handle1 OPTIONAL,
- IN EFI_HANDLE *Handle2 OPTIONAL,
+ IN EFI_HANDLE Handle1 OPTIONAL,
+ IN EFI_HANDLE Handle2 OPTIONAL,
IN CONST BOOLEAN Recursive,
IN CONST BOOLEAN Output
)
--=20
2.19.1.3.g30247aa5d201

[PATCH 27/35] SecurityPkg: stop abusing EFI_EVENT for protocol notify registration

Laszlo Ersek
 

EfiCreateProtocolNotifyEvent() takes a (VOID**) for "Registration",
similarly to gBS->RegisterProtocolNotify(). We should pass the address of
an actual pointer-to-VOID, and not the address of an EFI_EVENT. EFI_EVENT
just happens to be specified as (VOID*), and has nothing to do with the
registration.

This change is a no-op in practice; it's a semantic improvement.

Cc: Chao Zhang <chao.b.zhang@...>
Cc: Jian Wang <jian.j.wang@...>
Cc: Jiewen Yao <jiewen.yao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

SecurityPkg/HddPassword/HddPasswordDxe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c b/SecurityPkg/HddPa=
ssword/HddPasswordDxe.c
index b0d795b6597f..051e64091d7f 100644
--- a/SecurityPkg/HddPassword/HddPasswordDxe.c
+++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
@@ -2770,7 +2770,7 @@ HddPasswordDxeInit (
{
EFI_STATUS Status;
HDD_PASSWORD_DXE_PRIVATE_DATA *Private;
- EFI_EVENT Registration;
+ VOID *Registration;
EFI_EVENT EndOfDxeEvent;
EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
=20
--=20
2.19.1.3.g30247aa5d201

[PATCH 26/35] SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls

Laszlo Ersek
 

Unlike the InstallMultipleProtocolInterfaces() boot service, which takes
an (EFI_HANDLE*) as first parameter, the
UninstallMultipleProtocolInterfaces() boot service takes an EFI_HANDLE as
first parameter.

These are actual bugs. They must have remained hidden until now because
they are all in Unload() functions, which are probably exercised
infrequently. Fix the UninstallMultipleProtocolInterfaces() calls.

Cc: Chao Zhang <chao.b.zhang@...>
Cc: Jian Wang <jian.j.wang@...>
Cc: Jiewen Yao <jiewen.yao@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c =
| 2 +-
SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c =
| 2 +-
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDr=
iver.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/=
Tcg/Tcg2Config/Tcg2ConfigDriver.c
index 54155a338100..9052eced757d 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
@@ -443,7 +443,7 @@ Tcg2ConfigDriverUnload (
ASSERT (PrivateData->Signature =3D=3D TCG2_CONFIG_PRIVATE_DATA_SIGNATU=
RE);
=20
gBS->UninstallMultipleProtocolInterfaces (
- &ImageHandle,
+ ImageHandle,
&gEfiCallerIdGuid,
PrivateData,
NULL
diff --git a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c b/SecurityPkg=
/Tcg/TcgConfigDxe/TcgConfigDriver.c
index 341879e4c4ba..fb06624fdb8f 100644
--- a/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
+++ b/SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c
@@ -138,7 +138,7 @@ TcgConfigDriverUnload (
ASSERT (PrivateData->Signature =3D=3D TCG_CONFIG_PRIVATE_DATA_SIGNATUR=
E);
=20
gBS->UninstallMultipleProtocolInterfaces (
- &ImageHandle,
+ ImageHandle,
&gEfiCallerIdGuid,
PrivateData,
NULL
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/Secure=
BootConfigDriver.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDx=
e/SecureBootConfigDriver.c
index 798ef9cfbc01..6c0294151e6c 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon=
figDriver.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCon=
figDriver.c
@@ -115,7 +115,7 @@ SecureBootConfigDriverUnload (
ASSERT (PrivateData->Signature =3D=3D SECUREBOOT_CONFIG_PRIVATE_DATA_S=
IGNATURE);
=20
gBS->UninstallMultipleProtocolInterfaces (
- &ImageHandle,
+ ImageHandle,
&gEfiCallerIdGuid,
PrivateData,
NULL
--=20
2.19.1.3.g30247aa5d201

[PATCH 25/35] OvmfPkg/VideoDxe: document EFI_EDID_OVERRIDE_PROTOCOL.GetEdid() call

Laszlo Ersek
 

According to the UEFI spec -- and to the edk2 header
"MdePkg/Include/Protocol/EdidOverride.h" too --,
EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID takes an (EFI_HANDLE*), and not an
EFI_HANDLE, as second parameter ("ChildHandle").

This is probably [*] a bug in the UEFI spec. Given that this CSM module
(VideoDxe) had been used for a long time on physical platforms before it
was moved to OvmfPkg, keep the current "ChildHandle" argument, just cast
it explicitly.

[*] The edk2 tree contains no other GetEdid() call, and also no GetEdid()
implementation.

The edk2-platforms tree contains two GetEdid() calls, at commit
022c212167e0, in files
- "Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c",
- "Drivers/OptionRomPkg/CirrusLogic5430Dxe/Edid.c".

From these, the first passes an (EFI_HANDLE*) as "ChildHandle", the
second passes an EFI_HANDLE. It's difficult to draw a conclusion. :/

No functional changes.

(I've also requested a non-normative (informative) clarification for the
UEFI spec: <https://mantis.uefi.org/mantis/view.php?id=3D2018>, in the
direction that matches this patch.)

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: David Woodhouse <dwmw2@...>
Cc: Jordan Justen <jordan.l.justen@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
build-tested only

OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c b/OvmfPkg/Csm/Bio=
sThunk/VideoDxe/BiosVideo.c
index 0640656dba14..995136adee27 100644
--- a/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
+++ b/OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c
@@ -1402,9 +1402,13 @@ BiosVideoCheckForVbe (
goto Done;
}
=20
+ //
+ // Cast "ChildHandle" to (EFI_HANDLE*) in order to work around the s=
pec bug
+ // in UEFI v2.8, reported as Mantis#2018.
+ //
Status =3D EdidOverride->GetEdid (
EdidOverride,
- BiosVideoPrivate->Handle,
+ (EFI_HANDLE *) BiosVideoPrivate->Handle,
&EdidAttributes,
&EdidOverrideDataSize,
(UINT8 **) &EdidOverrideDataBlock
--=20
2.19.1.3.g30247aa5d201

[PATCH 24/35] OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal functions

Laszlo Ersek
 

In the following call tree:

PlatformInit ()
mInstalledPackages =3D HiiAddPackages ()
GopInstalled ()
PopulateForm (PackageList =3D mInstalledPackages)
CreateResolutionOptions (PackageList)
HiiSetString (PackageList
HiiUpdateForm (PackageList)

PlatformDxe passes around an EFI_HII_HANDLE that (a) originates from
HiiAddPackages() and (b) is ultimately passed to HiiSetString() and
HiiUpdateForm(). The intermediate functions PopulateForm() and
CreateResolutionOptions() however take that parameter as an
(EFI_HII_HANDLE*).

There is no bug in practice (because the affected functions never try to
de-reference the "PackageList" parameter, they just pass it on), but the
function prototypes are semantically wrong. Fix that.

This could remain hidden so long because pointer-to-VOID silently convert=
s
to/from any pointer-to-object type, and the UEFI spec mandates that
EFI_HII_HANDLE be a typedef to (VOID*).

Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Jordan Justen <jordan.l.justen@...>
Signed-off-by: Laszlo Ersek <lersek@...>
---

Notes:
tested in UiApp

OvmfPkg/PlatformDxe/Platform.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platfor=
m.c
index 09181769babf..23ad43901f66 100644
--- a/OvmfPkg/PlatformDxe/Platform.c
+++ b/OvmfPkg/PlatformDxe/Platform.c
@@ -486,7 +486,7 @@ STATIC
EFI_STATUS
EFIAPI
CreateResolutionOptions (
- IN EFI_HII_HANDLE *PackageList,
+ IN EFI_HII_HANDLE PackageList,
OUT VOID **OpCodeBuffer,
IN UINTN NumGopModes,
IN GOP_MODE *GopModes
@@ -547,7 +547,7 @@ STATIC
EFI_STATUS
EFIAPI
PopulateForm (
- IN EFI_HII_HANDLE *PackageList,
+ IN EFI_HII_HANDLE PackageList,
IN EFI_GUID *FormSetGuid,
IN EFI_FORM_ID FormId,
IN UINTN NumGopModes,
--=20
2.19.1.3.g30247aa5d201