[edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
Laszlo Ersek
lersek at redhat.com
Tue Sep 17 19:49:01 UTC 2019
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 at arm.com>
Cc: Andrew Fish <afish at apple.com>
Cc: Anthony Perard <anthony.perard at citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
Cc: Benjamin You <benjamin.you at intel.com>
Cc: Chao Zhang <chao.b.zhang at intel.com>
Cc: Dandan Bi <dandan.bi at intel.com>
Cc: David Woodhouse <dwmw2 at infradead.org>
Cc: Eric Dong <eric.dong at intel.com>
Cc: Guo Dong <guo.dong at intel.com>
Cc: Hao A Wu <hao.a.wu at intel.com>
Cc: Jaben Carsey <jaben.carsey at intel.com>
Cc: Jian J Wang <jian.j.wang at intel.com>
Cc: Jian Wang <jian.j.wang at intel.com>
Cc: Jiaxin Wu <jiaxin.wu at intel.com>
Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Jordan Justen <jordan.l.justen at intel.com>
Cc: Julien Grall <julien.grall at arm.com>
Cc: Leif Lindholm <leif.lindholm at linaro.org>
Cc: Liming Gao <liming.gao at intel.com>
Cc: Maurice Ma <maurice.ma at intel.com>
Cc: Michael D Kinney <michael.d.kinney at intel.com>
Cc: Ray Ni <ray.ni at intel.com>
Cc: Siyuan Fu <siyuan.fu at intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>
Cc: Zhichao Gao <zhichao.gao at intel.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list