From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 17 Sep 2019 12:49:51 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 4CF1F308FC4E; Tue, 17 Sep 2019 19:49:50 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-37.rdu2.redhat.com [10.10.120.37]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8DDE4600C8; Tue, 17 Sep 2019 19:49:44 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Achin Gupta , Andrew Fish , Anthony Perard , Ard Biesheuvel , Benjamin You , Chao Zhang , Dandan Bi , David Woodhouse , Eric Dong , Guo Dong , Hao A Wu , Jaben Carsey , Jian J Wang , Jiaxin Wu , Jiewen Yao , Jordan Justen , Julien Grall , Leif Lindholm , Liming Gao , Maurice Ma , Michael D Kinney , Ray Ni , Siyuan Fu , Supreeth Venkatesh , Zhichao Gao Subject: [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID Date: Tue, 17 Sep 2019 21:49:01 +0200 Message-Id: <20190917194935.24322-2-lersek@redhat.com> In-Reply-To: <20190917194935.24322-1-lersek@redhat.com> References: <20190917194935.24322-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Tue, 17 Sep 2019 19:49:50 +0000 (UTC) Content-Transfer-Encoding: quoted-printable Unfortunately, the UEFI / PI / Shell specs define a number of handle type= s 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 o= f 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-specifi= c type) to the typedef. Some of these also demonstrate how the code could have looked in practice if the specs had used proper opaque (=3Dincomplet= e) 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 Cc: Andrew Fish Cc: Anthony Perard Cc: Ard Biesheuvel Cc: Benjamin You Cc: Chao Zhang Cc: Dandan Bi Cc: David Woodhouse Cc: Eric Dong Cc: Guo Dong Cc: Hao A Wu Cc: Jaben Carsey Cc: Jian J Wang Cc: Jian Wang Cc: Jiaxin Wu Cc: Jiewen Yao Cc: Jordan Justen Cc: Julien Grall Cc: Leif Lindholm Cc: Liming Gao Cc: Maurice Ma Cc: Michael D Kinney Cc: Ray Ni Cc: Siyuan Fu Cc: Supreeth Venkatesh Cc: Zhichao Gao Signed-off-by: Laszlo Ersek --- 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; =20 /// /// 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; =20 /// /// Declare the forward reference data structure for EFI_PEI_SERVICE. diff --git a/MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h b/MdePk= g/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 }} =20 typedef UINT32 EFI_ACPI_TABLE_VERSION; -typedef VOID *EFI_ACPI_HANDLE; +struct EFI_ACPI_OBJECT; +typedef struct EFI_ACPI_OBJECT *EFI_ACPI_HANDLE; =20 #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; =20 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; =20 /// /// 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 } } =20 typedef struct _EFI_HII_FONT_PROTOCOL EFI_HII_FONT_PROTOCOL; - -typedef VOID *EFI_FONT_HANDLE; +typedef LIST_ENTRY *EFI_FONT_HANDLE; =20 /// /// EFI_HII_OUT_FLAGS. diff --git a/MdePkg/Include/Protocol/MmMp.h b/MdePkg/Include/Protocol/MmM= p.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; =20 typedef struct { MM_COMPLETION Completion; diff --git a/MdePkg/Include/Protocol/S3SaveState.h b/MdePkg/Include/Proto= col/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, 0= xa8, 0x87 }} =20 =20 -typedef VOID *EFI_S3_BOOT_SCRIPT_POSITION; +typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION; =20 typedef struct _EFI_S3_SAVE_STATE_PROTOCOL EFI_S3_SAVE_STATE_PROTOCOL; =20 diff --git a/MdePkg/Include/Protocol/Shell.h b/MdePkg/Include/Protocol/Sh= ell.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__ =20 #include +#include =20 #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; =20 typedef enum { /// diff --git a/MdePkg/Include/Protocol/UserManager.h b/MdePkg/Include/Proto= col/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, 0x= f6, 0x83 } \ } =20 -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; =20 /// /// The attributes of the user profile information. @@ -157,7 +159,8 @@ typedef CHAR16 *EFI_USER_INFO_CREDENTIAL_PROVIDER_NAM= E; /// 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 b= e considered a match. /// diff --git a/MdePkg/Include/Uefi/UefiBaseType.h b/MdePkg/Include/Uefi/Uef= iBaseType.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/MdePk= g/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; =20 #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/MdeModu= lePkg/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/MdeMod= ulePkg/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 { =20 #define HII_HANDLE_SIGNATURE SIGNATURE_32 ('h','i','h','l') =20 -typedef struct { +typedef struct EFI_HII_OBJECT { UINTN Signature; LIST_ENTRY Handle; UINTN Key; diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Co= re/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; --=20 2.19.1.3.g30247aa5d201