Date   

Re: [PATCH 15/35] MdeModulePkg/PiSmmCore: make type punning consistent

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@intel.com>

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, September 18, 2019 3:49 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Dong, Eric <eric.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
Subject: [PATCH 15/35] MdeModulePkg/PiSmmCore: make type punning
consistent

The SmiHandlerRegister() function explicitly casts "SmiHandler" (of type
(SMI_HANDLER*)) to EFI_HANDLE, when outputting "DispatchHandle".

Apply the same cast in the counterpart function SmiHandlerUnRegister(),
which compares multiple "SmiHandler"s against the input "DispatchHandle".

This is a semantic cleanup; there is no functional change.

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

Notes:
build-tested only, most likely -- I'm unaware of any code paths in OVMF
that would lead to SmiHandlerUnRegister()

MdeModulePkg/Core/PiSmmCore/Smi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
b/MdeModulePkg/Core/PiSmmCore/Smi.c
index f8bd9f49ee3c..488af6754faf 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -282,7 +282,7 @@ SmiHandlerUnRegister (
//
SmiHandler = NULL;
for ( HandlerLink = GetFirstNode (&mRootSmiEntry.SmiHandlers)
- ; !IsNull (&mRootSmiEntry.SmiHandlers, HandlerLink) && (SmiHandler !=
DispatchHandle)
+ ; !IsNull (&mRootSmiEntry.SmiHandlers, HandlerLink) &&
+ ((EFI_HANDLE) SmiHandler != DispatchHandle)
; HandlerLink = GetNextNode (&mRootSmiEntry.SmiHandlers, HandlerLink)
) {
SmiHandler = CR (HandlerLink, SMI_HANDLER, Link,
SMI_HANDLER_SIGNATURE); @@ -292,19 +292,19 @@
SmiHandlerUnRegister (
// Look for it in non-root SMI handlers
//
for ( EntryLink = GetFirstNode (&mSmiEntryList)
- ; !IsNull (&mSmiEntryList, EntryLink) && (SmiHandler != DispatchHandle)
+ ; !IsNull (&mSmiEntryList, EntryLink) && ((EFI_HANDLE) SmiHandler
+ != DispatchHandle)
; EntryLink = GetNextNode (&mSmiEntryList, EntryLink)
) {
SmiEntry = CR (EntryLink, SMI_ENTRY, AllEntries, SMI_ENTRY_SIGNATURE);
for ( HandlerLink = GetFirstNode (&SmiEntry->SmiHandlers)
- ; !IsNull (&SmiEntry->SmiHandlers, HandlerLink) && (SmiHandler !=
DispatchHandle)
+ ; !IsNull (&SmiEntry->SmiHandlers, HandlerLink) &&
+ ((EFI_HANDLE) SmiHandler != DispatchHandle)
; HandlerLink = GetNextNode (&SmiEntry->SmiHandlers, HandlerLink)
) {
SmiHandler = CR (HandlerLink, SMI_HANDLER, Link,
SMI_HANDLER_SIGNATURE);
}
}

- if (SmiHandler != DispatchHandle) {
+ if ((EFI_HANDLE) SmiHandler != DispatchHandle) {
return EFI_INVALID_PARAMETER;
}

--
2.19.1.3.g30247aa5d201


[PATCH v3] MinPlatformPkg/TestPointCheckLib: Add check for pointers

Zhang, Shenglei
 

In DxeCheckBootVariable.c, add check for BootOrder and Variable
that return EFI_NOT_FOUND when they are NULL.
In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL
when allocating memory to what it points to.

Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---

v2: Update copyright

v3: Fix the coding style.

.../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++++++-
.../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 8 +++++---
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
index 85bd5b3d..f658beb7 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -130,6 +130,9 @@ TestPointCheckLoadOptionVariable (
for (ListIndex = 0; ListIndex < sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]); ListIndex++) {
UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder", mLoadOptionVariableList[ListIndex]);
Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid, (VOID **)&BootOrder, &OrderSize);
+ if(BootOrder == NULL) {
+ return EFI_NOT_FOUND;
+ }
if (EFI_ERROR(Status)) {
continue;
}
@@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable (
for (Index = 0; ; Index++) {
UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", mKeyOptionVariableList[ListIndex], Index);
Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, &Variable, &Size);
+ if(Variable == NULL) {
+ return EFI_NOT_FOUND;
+ }
if (!EFI_ERROR(Status)) {
DumpKeyOption (KeyOptionName, Variable, Size);
} else {
diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
index 82709d44..28ca8382 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -241,11 +241,13 @@ TestPointDumpGcd (
}
}
if (GcdMemoryMap != NULL) {
- *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ if (GcdIoMap != NULL) {
+ *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ }
*GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
}
}
-
+
if (DumpPrint) {
DEBUG ((DEBUG_INFO, "==== TestPointDumpGcd - Exit\n"));
}
--
2.18.0.windows.1


Re: [PATCH v2 0/2] *** Add VS2019 Support ***

Cheng, Ching JenX
 

Hi Liming,

The VS2017 is still working fine with this changes,
I have verified it with VS2019, VS2017 and VS2015 build,

Thanks,
Allen

-----Original Message-----
From: Gao, Liming
Sent: Tuesday, September 17, 2019 10:57 PM
To: devel@edk2.groups.io; Cheng, Ching JenX
<ching.jenx.cheng@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 0/2] *** Add VS2019 Support ***

Ching:
The change is good. With this change, have you verified VS2017 tool chain?
I want to make sure there is no impact on VS2017.

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Cheng, Ching JenX
Sent: Tuesday, September 17, 2019 11:23 AM
To: devel@edk2.groups.io; Cheng, Ching JenX
<ching.jenx.cheng@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 0/2] *** Add VS2019 Support ***

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2182

In order to support VS2019 on EDK2, the following patches was modified
def and batch files 1. Add VS2019 x86/64 definitions on tools_def.template
2. Add VS2019 support on toolsetup batches, and add version check with
command vswhere
Because VS2019 and VS2017 using the same vswhere to get the
InstallationPath

v2: In 01/02, add ARM/AARCH64/EBC Definitions, Combine VS2017_HOST
and
VS2019_HOST to VS_HOST

Ching JenX Cheng (2):
Add VS2019 Toolchain def
Add VS2019 Support on ToolSetup Batches

BaseTools/Conf/tools_def.template | 220
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++++++++++++++++++++-------------------
BaseTools/get_vsvars.bat | 37
++++++++++++++++++++++++++++++-------
BaseTools/set_vsprefix_envs.bat | 70
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+++++++-----
BaseTools/toolsetup.bat | 16 +++++++++++++---
edksetup.bat | 6 ++++--
5 files changed, 313 insertions(+), 36 deletions(-)

--
2.21.0.windows.1






Re: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

Wu, Hao A
 

Hello Tyler,

 

I agree that the UEFI spec can be refined to explicitly mention the below 2 fields:

 

* Input parameter 'NamespaceId' for the PassThru() service

* The 'Nsid' field of the EFI_NVM_EXPRESS_COMMAND

 

should match for the ‘PassThru’ service.

 

Best Regards,

Hao Wu

 

From: Tyler J Erickson [mailto:tyler.j.erickson@...]
Sent: Wednesday, September 18, 2019 12:50 AM
To: Wu, Hao A
Cc: devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

 

Hi Hao,

 

Sorry for the late reply.

I tested and can confirm that if the NSID of the command and the provided NamespaceID as an input match, the commands do go through on the EDK2 driver. This is not the case with a driver built into a motherboard that I have here, as it only accepts values for the current NSID and nothing else. This is ok since I can load the EDK2 driver in place instead.

 

Since this is unclear in the documentation for the passthru function, can we make a documentation change instead?

 

Thanks,

-Tyler

 

 

On Wed, Sep 4, 2019 at 8:07 PM Wu, Hao A <hao.a.wu@...> wrote:

Hello Tyler,

 

Some inline comments below.

 

Best Regards,

Hao Wu

 

From: Tyler J Erickson [mailto:tyler.j.erickson@...]
Sent: Wednesday, September 04, 2019 11:07 PM
To: Wu, Hao A
Cc: devel@edk2.groups.io; Ni, Ray
Subject: Re: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs for Admin commands

 

Hi Hao,

 

I tried making both the NamespaceID and NSID values the same when calling the passthru function for these admin commands and it didn't work. I think that is due to another place in the passthru code filtering the NamespaceId input values on line 517-520.

The NamespaceId parameter is being checked to make sure it isn't greater than the number of supported namespaces by the controller and it makes sure it isn't set to (UINT32)-1 (All F's). 

 

 

[Hao]: I think the check on line 517-520 blocks ‘NamespaceId’ with value greater than the number of namespace. But it allows the value to be MAX_UINT32, which means the command takes effect on all namespaces.

 

 

I think this is correct since the NamespaceId input is what is discovered when calling the EFI_NVM_EXPRESS_PASS_THRU_GET_NEXT_NAMESPACE function. This is what I used to get the available namespaces available in the system.

 

In my case I'm sending some commands in my application with the NSID set to UINT32_MAX in order to request controller wide SMART/Health log data among other things. There are some commands where setting the NSID to another value may also be useful. For example, DST can use the NSID in the command to change between testing only the controller (0), testing all namespaces (UINT32_MAX), and a specific namespace.

 

 

[Hao]: Yes. Just as you said, commands like DST or Get Log Page will have different target when different Namespace values are provided.

I think for your case, you can:

1.       Set both the 'NamespaceId' parameter and 'Nsid' field to 0 if the target is the controller;

2.       Set both of them MAX_UINT32 if the target is all the namespaces;

3.       Set both of them to a corresponding ID if the target is a specific namespace.

 

As you can see from the current implementation of NvmExpressPassThru() function, the 'NamespaceId' parameter does not affect the construct of the submission queue item.

The 'NamespaceId' parameter only serves as a valid check for the 'Nsid' field in the EFI_NVM_EXPRESS_COMMAND structure.

 

 

The modification I made was done where the passthru command's NSID was actually being checked, which seemed like the best place to add this exception for admin commands.


-Tyler

 

 

On Tue, Sep 3, 2019 at 9:39 PM Wu, Hao A <hao.a.wu@...> wrote:

> -----Original Message-----
> From: Wu, Hao A
> Sent: Wednesday, September 04, 2019 11:39 AM
> To: devel@edk2.groups.io; Tyler Erickson
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Repost the mail to the list.
>
> Best Regards,
> Hao Wu
>
> -----Original Message-----
> From: Tyler Erickson [mailto:tyler.j.erickson@...]
> Sent: Tuesday, September 03, 2019 9:55 PM
> To: edk2-devel@...
> Cc: Wu, Hao A; Ni, Ray
> Subject: [PATCH v1 1/1] MdeModulePkg/NvmExpressDxe: Allow other NSIDs
> for Admin commands
>
> Cc: Hao A Wu <hao.a.wu@...>
> Cc: Ray Ni <ray.ni@...>
> Signed-off-by: Tyler Erickson <tyler.j.erickson@...>
> ---
>  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> index 8e721379466a..78a3c663ded4 100644
> --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c
> @@ -561,7 +561,7 @@ NvmExpressPassThru (
>    Sq  = Private->SqBuffer[QueueId] + Private->SqTdbl[QueueId].Sqt;
>    Cq  = Private->CqBuffer[QueueId] + Private->CqHdbl[QueueId].Cqh;
>
> -  if (Packet->NvmeCmd->Nsid != NamespaceId) {
> +  if (Packet->QueueType != NVME_ADMIN_QUEUE && Packet->NvmeCmd-
> >Nsid != NamespaceId) {


Hello,

Per my understanding to the codes, I think the:

* Input parameter 'NamespaceId' for the PassThru() service
* The 'Nsid' field of the EFI_NVM_EXPRESS_COMMAND

are of the same meaning.

Do you think setting these 2 values identical when calling the PassThru()
service can resolve the issue you met?

Best Regards,
Hao Wu


>      return EFI_INVALID_PARAMETER;
>    }
>
> --
> 2.17.1


[Patch V3] UefiCpuPkg/CpuExceptionHandlerLib: Fix split lock

John E Lofgren
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
V3 changes:
change to mov instruction (non locking instuction) instead
of xchg to simplify design.

V2 changes:
Add xchg 16 bit instructions to handle sgdt and sidt base
63:48 bits and 47:32 bits.
Add comment to explain why xchg 64bit isnt being used

Split lock happens when a locking instruction is used on mis-aligned data
that crosses two cachelines. If close source platform enables Alignment Check
Exception(#AC), They can hit a double fault due to split lock being in
CpuExceptionHandlerLib.

sigt and sgdt saves 10 bytes to memory, 8 bytes is base and 2 bytes is limit.
The data is mis-aligned, can cross two cacheline, and a xchg
instruction(locking instuction) is being utilize.

Signed-off-by: John E Lofgren <john.e.lofgren@intel.com>
---
UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
index 4db1a09f28..7b7642b290 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
@@ -180,21 +180,29 @@ HasErrorCode:
push qword [rbp + 24]

;; UINT64 Gdtr[2], Idtr[2];
+ ; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes = limit.
+ ; To avoid #AC split lock when separating base and limit into their
+ ; own separate 64 bit memory, we can’t use 64 bit xchg since base [63:48] bits
+ ; may cross the cache line.
xor rax, rax
push rax
push rax
sidt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ xchg eax, [rsp + 2]
+ xchg eax, [rsp]
+ xchg eax, [rsp + 8]
+ xchg ax, [rsp + 6]
+ xchg ax, [rsp + 4]

xor rax, rax
push rax
push rax
sgdt [rsp]
- xchg rax, [rsp + 2]
- xchg rax, [rsp]
- xchg rax, [rsp + 8]
+ xchg eax, [rsp + 2]
+ xchg eax, [rsp]
+ xchg eax, [rsp + 8]
+ xchg ax, [rsp + 6]
+ xchg ax, [rsp + 4]

;; UINT64 Ldtr, Tr;
xor rax, rax
--
2.16.2.windows.1


Updated Event: TianoCore Design Meeting - APAC/NAMO #cal-invite

devel@edk2.groups.io Calendar <devel@...>
 

TianoCore Design Meeting - APAC/NAMO

When:
Thursday, 4 April 2019
6:30pm to 7:30pm
(UTC-07:00) America/Los Angeles
Repeats: Every 2 weeks on Thursday

Where:
BlueJeans Meeting

Organizer: Stephano Cetola stephano.cetola@...

Description:

For more info, see here:

https://www.tianocore.org/design-meeting/

 

To join the meeting on a computer or mobile phone:

https://bluejeans.com/889357567?src=calendarLink

 

Phone Dial-in

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

Global Numbers: https://www.bluejeans.com/numbers

 

Meeting ID: 889 357 567

 

Room System

199.48.152.152 or bjn.vc

 

Meeting ID: 889 357 567

 

Want to test your video connection?

https://bluejeans.com/111


Re: [PATCH v2] MinPlatformPkg/TestPointCheckLib: Add return value when OutTable is NULL

Nate DeSimone
 

Hi Shenglei,

Your explanation makes sense.

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

Thanks,
Nate

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: Monday, September 16, 2019 7:48 PM
To: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH v2] MinPlatformPkg/TestPointCheckLib: Add return value when OutTable is NULL

Hi Nathaniel,

Thanks for your comments and below is my response.

-----Original Message-----
From: Desimone, Nathaniel L
Sent: Tuesday, September 17, 2019 3:25 AM
To: Zhang, Shenglei <shenglei.zhang@intel.com>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
<chasel.chiu@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [PATCH v2] MinPlatformPkg/TestPointCheckLib: Add return
value when OutTable is NULL

Hi Shenglei,

I don't see how this patch is at all related to the previous version of this patch.
What is different from the previous version is that this patch update the copyright year and the "if...else" coding style.

Also, you are introducing yet another new bug with this patch.
Moreover, this bug is unrelated to the previous bug.

Please take a look at the function TestPointGetAcpi(). With your
change added, this function is now broken since Table is a stack
variable and it is not being initialized to zero. This function
assumes that
DumpAcpiXsdt()/DumpAcpiRsdt() will do the initialization to zero on
it's behalf, you have broken this assumption with your change.
I have taken a look at the function TestPointGetAcpi(). The variable Table is first initialized to NULL by DumpAcpiXsdt()/DumpAcpiRsdt(). The reference code is "*OutTable = NULL"(line 667). And it will be assigned a value next. The reference code is "*OutTable = Table"(line 689).

And with my changes, the stack variable Table(equivalent to *OutTable in DumpAcpiXsdt/ DumpAcpiRsdt) can still be initialized to NULL or assigned a value. What I did is intended to check the address of "Table", since there is no point to perform operations to "Table" if its address is NULL.

Thanks,
Shenglei


Both this patch and the previous patch have been made carelessly and I
am not impressed.

Thanks,
Nate

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: Sunday, September 15, 2019 6:09 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
<chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH v2] MinPlatformPkg/TestPointCheckLib: Add return value
when OutTable is NULL

Currently there is no check for the parameter OutTable.
So add the logic that return value EFI_INVALID_PARAMETER when the
OutTable is NULL.

Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---

v2:Update the copyright and the if...else statement coding style.

.../Test/Library/TestPointCheckLib/DxeCheckAcpi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Acpi.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Acpi.c
index 263781a2..83736bf3 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Acpi.c
+++
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckAcpi.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2019, Intel Corporation. All rights
+reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -610,6 +610,8 @@ DumpAcpiRsdt (

if (OutTable != NULL) {
*OutTable = NULL;
+ } else{
+ return EFI_INVALID_PARAMETER;
}

ReturnStatus = EFI_SUCCESS;
@@ -663,6 +665,8 @@ DumpAcpiXsdt (

if (OutTable != NULL) {
*OutTable = NULL;
+ } else{
+ return EFI_INVALID_PARAMETER;
}

ReturnStatus = EFI_SUCCESS;
--
2.18.0.windows.1


Updated Event: TianoCore Bug Triage - APAC / NAMO #cal-invite

devel@edk2.groups.io Calendar <devel@...>
 

TianoCore Bug Triage - APAC / NAMO

When:
Thursday, 4 April 2019
5:00pm to 5:30pm
(UTC-07:00) America/Los Angeles
Repeats: Every 2 weeks on Thursday

Where:
BlueJeans Meeting

Organizer: Stephano Cetola stephano.cetola@...

Description:

For more info please see:

https://www.tianocore.org/bug-triage

 

To join the meeting on a computer or mobile phone:

https://bluejeans.com/889357567?src=calendarLink

 

Phone Dial-in

+1.408.740.7256 (US (San Jose))

+1.408.317.9253 (US (Primary, San Jose))

Global Numbers: https://www.bluejeans.com/numbers

 

Meeting ID: 889 357 567

 

Room System

199.48.152.152 or bjn.vc

 

Meeting ID: 889 357 567

 

Want to test your video connection?

https://bluejeans.com/111

 

 


Cancelled Event: TianoCore Design / Bug Triage - EMEA - Wednesday, 18 September 2019 #cal-cancelled

devel@edk2.groups.io Calendar <devel@...>
 

Cancelled: TianoCore Design / Bug Triage - EMEA

This event has been cancelled.

When:
Wednesday, 18 September 2019
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

Where:
https://zoom.us/j/695893389

Organizer: Stephano Cetola stephano.cetola@...

Description:

Join Zoom Meeting

https://zoom.us/j/695893389

 

One tap mobile

+17207072699,,695893389# US

+16465588656,,695893389# US (New York)

 

Dial by your location

        +1 720 707 2699 US

        +1 646 558 8656 US (New York)

Meeting ID: 695 893 389

Find your local number: https://zoom.us/u/abOtdJckxL

 


Re: [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers

Nate DeSimone
 

Hi Shenglei,

You are right, *GcdIoMap will always be NULL but GcdIoMap may not be as the function does not exit early in the case of GcdIoMap being == NULL, sorry I misread. Please make sure you put a space between your closing parenthesis and your opening curly:

if (GcdIoMap != NULL) {

not...

if (GcdIoMap != NULL){

Also, please fix the double semicolon and if statement whitespace:

If (BootOrder == NULL) {
return EFI_NOT_FOUND;
}

not...

if(BootOrder == NULL) {
return EFI_NOT_FOUND;;
}

Then you will get a reviewed-by.

Thanks,
Nate

-----Original Message-----
From: Zhang, Shenglei <shenglei.zhang@intel.com>
Sent: Monday, September 16, 2019 8:36 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: RE: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers

Hi Nathaniel,

-----Original Message-----
From: Desimone, Nathaniel L
Sent: Saturday, September 14, 2019 6:31 AM
To: Zhang, Shenglei <shenglei.zhang@intel.com>; devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
<chasel.chiu@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib:
Add check for pointers

Hi Shenglei,

Looking at this patch more closely. There appear to be bugs... please
see below. Please fix this along with your poor use of semi-colons and
white-space.

Thanks,

Nate

On 9/12/2019 11:54 AM, Nate DeSimone wrote:
Your whitespace doesn't quite match the edk2 coding style
guidelines, but
we can fix that during commit.

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Zhang, Shenglei
Sent: Wednesday, September 11, 2019 7:55 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A <michael.a.kubacki@intel.com>; Chiu, Chasel
<chasel.chiu@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desimone@intel.com>; Gao, Liming <liming.gao@intel.com>
Subject: [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for
pointers

In DxeCheckBootVariable.c, add check for BootOrder and Variable that
return EFI_NOT_FOUND when they are NULL.
In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when
allocating memory to what it points to.

Cc: Michael Kubacki <michael.a.kubacki@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
---

v2: Update copyright

.../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++++++-
.../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 ++++--
2 files changed, 11 insertions(+), 3 deletions(-)

diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
BootVariable.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
BootVariable.c
index 85bd5b3d..98130683 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
BootVariable.c
+++
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckBootVariable.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights
+reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -130,6 +130,9 @@ TestPointCheckLoadOptionVariable (
for (ListIndex = 0; ListIndex <
sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]);
ListIndex++) {
UnicodeSPrint (BootOrderName, sizeof(BootOrderName),
L"%sOrder",
mLoadOptionVariableList[ListIndex]);
Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid,
(VOID **)&BootOrder, &OrderSize);
+ if(BootOrder == NULL) {
+ return EFI_NOT_FOUND;;
+ }
if (EFI_ERROR(Status)) {
continue;
}
@@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable (
for (Index = 0; ; Index++) {
UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName),
L"%s%04x",
mKeyOptionVariableList[ListIndex], Index);
Status = GetVariable2 (KeyOptionName,
&gEfiGlobalVariableGuid,
&Variable, &Size);
+ if(Variable == NULL) {
+ return EFI_NOT_FOUND;;
+ }
if (!EFI_ERROR(Status)) {
DumpKeyOption (KeyOptionName, Variable, Size);
} else {
diff --git
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Gcd.c
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Gcd.c
index 82709d44..c90b37f2 100644
---
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeChec
k
Gcd.c
+++
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckGcd.c
@@ -1,6 +1,6 @@
/** @file

-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017-2019, Intel Corporation. All rights
+reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -241,7 +241,9 @@ TestPointDumpGcd (
}
}
if (GcdMemoryMap != NULL) {
- *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ if (GcdIoMap != NULL){
+ *GcdIoMap = AllocateCopyPool (NumberOfDescriptors *
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+ }
GcdIoMap will always be NULL. Please see line 199 of this file. I
believe your patch is introducing a new bug.
I fail to get your point. Would you explain why GcdIoMap will always be NULL? It looks like an input parameter from other place.

Thanks,
Shenglei


*GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
}
}
--
2.18.0.windows.1




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

Michael D Kinney
 

Andrew,

Perhaps we want strict type checking as default, and platforms
Or packages that can not build with strict type checking set
the define for the relaxed type checking in their DSC files.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On
Behalf Of Andrew Fish via Groups.Io
Sent: Tuesday, September 17, 2019 2:07 PM
To: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Cc: lersek@redhat.com; Achin Gupta
<achin.gupta@arm.com>; Anthony Perard
<anthony.perard@citrix.com>; Ard Biesheuvel
<ard.biesheuvel@linaro.org>; You, Benjamin
<benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi, Dandan
<dandan.bi@intel.com>; David Woodhouse
<dwmw2@infradead.org>; Dong, Eric
<eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen, Jordan L
<jordan.l.justen@intel.com>; Julien Grall
<julien.grall@arm.com>; Leif Lindholm
<leif.lindholm@linaro.org>; Gao, Liming
<liming.gao@intel.com>; Ma, Maurice
<maurice.ma@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Fu, Siyuan
<siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao
<zhichao.gao@intel.com>
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:28 PM, Ni, Ray
<ray.ni@intel.com> wrote:

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. 😊
Ray,

I'm flexible on the name, I was just trying to give a
good example of what I was suggesting.

Thanks,

Andrew Fish

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@intel.com>
Cc: devel@edk2.groups.io; lersek@redhat.com; Achin
Gupta
<achin.gupta@arm.com>; Anthony Perard
<anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You,
Benjamin
<benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi,
Dandan <dandan.bi@intel.com>; David Woodhouse
<dwmw2@infradead.org>;
Dong, Eric <eric.dong@intel.com>; Dong, Guo
<guo.dong@intel.com>; Wu,
Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>;
Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; Julien Grall
<julien.grall@arm.com>; Leif Lindholm
<leif.lindholm@linaro.org>;
Gao, Liming <liming.gao@intel.com>; Ma, Maurice
<maurice.ma@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Fu, Siyuan
<siyuan.fu@intel.com>;
Supreeth Venkatesh <supreeth.venkatesh@arm.com>;
Gao, Zhichao
<zhichao.gao@intel.com>
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@intel.com> 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@arm.com>; Andrew Fish
<afish@apple.com>; Anthony Perard
<anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You,
Benjamin
<benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>;
Bi, Dandan <dandan.bi@intel.com>; David Woodhouse
<dwmw2@infradead.org>; Dong,
Eric
<eric.dong@intel.com>; Dong, Guo
<guo.dong@intel.com>; Wu, Hao A
<hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang,
Jian J <jian.j.wang@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan
L
<jordan.l.justen@intel.com>; Julien Grall
<julien.grall@arm.com>;
Leif
Lindholm
<leif.lindholm@linaro.org>; Gao, Liming
<liming.gao@intel.com>; Ma,
Maurice <maurice.ma@intel.com>; Kinney,
Michael
D <michael.d.kinney@intel.com>; Ni, Ray
<ray.ni@intel.com>; Fu,
Siyuan <siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao
<zhichao.gao@intel.com>
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@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh
<supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@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/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[ray.ni@intel.com] -=-=-=-=-=-=





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:28 PM, Ni, Ray <ray.ni@intel.com> wrote:

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. 😊
Ray,

I'm flexible on the name, I was just trying to give a good example of what I was suggesting.

Thanks,

Andrew Fish

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@intel.com>
Cc: devel@edk2.groups.io; lersek@redhat.com; Achin Gupta <achin.gupta@arm.com>; Anthony Perard
<anthony.perard@citrix.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>;
Zhang, Chao B <chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse
<dwmw2@infradead.org>; Dong, Eric <eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A
<hao.a.wu@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall
<julien.grall@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice
<maurice.ma@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth
Venkatesh <supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@intel.com> 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@arm.com>; Andrew Fish <afish@apple.com>; Anthony Perard
<anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse <dwmw2@infradead.org>; Dong,
Eric
<eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien.grall@arm.com>; Leif
Lindholm
<leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Kinney,
Michael
D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@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/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=




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@intel.com>
Cc: devel@edk2.groups.io; lersek@redhat.com; Achin Gupta <achin.gupta@arm.com>; Anthony Perard
<anthony.perard@citrix.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>;
Zhang, Chao B <chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse
<dwmw2@infradead.org>; Dong, Eric <eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A
<hao.a.wu@intel.com>; Carsey, Jaben <jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin
<jiaxin.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall
<julien.grall@arm.com>; Leif Lindholm <leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice
<maurice.ma@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth
Venkatesh <supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@intel.com> 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@arm.com>; Andrew Fish <afish@apple.com>; Anthony Perard
<anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse <dwmw2@infradead.org>; Dong,
Eric
<eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien.grall@arm.com>; Leif
Lindholm
<leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Kinney,
Michael
D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@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/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=


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@intel.com> 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@arm.com>; Andrew Fish <afish@apple.com>; Anthony Perard <anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse <dwmw2@infradead.org>; Dong, Eric
<eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien.grall@arm.com>; Leif Lindholm
<leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@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/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=


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

Ni, Ray
 

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

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni, Ray
<ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
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@intel.com>

-----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@intel.com>; Dong, Eric <eric.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
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@intel.com]
-=-=-=-=-=-=


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

Ni, Ray
 

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

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>
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@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

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@arm.com>; Andrew Fish <afish@apple.com>; Anthony Perard <anthony.perard@citrix.com>;
Ard Biesheuvel <ard.biesheuvel@linaro.org>; You, Benjamin <benjamin.you@intel.com>; Zhang, Chao B
<chao.b.zhang@intel.com>; Bi, Dandan <dandan.bi@intel.com>; David Woodhouse <dwmw2@infradead.org>; Dong, Eric
<eric.dong@intel.com>; Dong, Guo <guo.dong@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
<jaben.carsey@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Yao, Jiewen
<jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Julien Grall <julien.grall@arm.com>; Leif Lindholm
<leif.lindholm@linaro.org>; Gao, Liming <liming.gao@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Supreeth Venkatesh
<supreeth.venkatesh@arm.com>; Gao, Zhichao <zhichao.gao@intel.com>
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@arm.com>
Cc: Andrew Fish <afish@apple.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jian Wang <jian.j.wang@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Laszlo Ersek <lersek@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/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=


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

Ni, Ray
 

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

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

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

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

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@intel.com>

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Tuesday, September 17, 2019 12:49 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Andrew Fish <afish@apple.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Ni, Ray <ray.ni@intel.com>
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@apple.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

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