Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
On 09/18/19 17:55, Andrew Fish wrote: > > >> On Sep 18, 2019, at 1:41 AM, Laszlo Ersek wrote: >> >> On 09/17/19 22:22, Andrew Fish wrote: >>> >>> On Sep 17, 2019, at 1:06 PM, Ni, Ray 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 >> >> Technically, this would work well. >> >> However, if we wanted to allow new projects to #define >> STRICTER_UEFI_TYPES as their normal mode of operation (and not just for >> a sanity check in CI), then we'd have to update the UEFI spec too. >> >> Otherwise, code that is technically spec-conformant (albeit semantically >> nonsensical), like I mentioned up-thread, would no longer compile: >> > > Laszlo, > > I think helping people NOT write nonsensical code is good. It is very good > idea and I'd like to add it to the spec but as you point out it would break a > lot of existing code so I'm not sure it is possible. I guess we could try to > add a strict mode to the spec but given the types are defined in tables that > may be problematic. > > We have coding standards that are more strict than what the C spec allows. So > I would see the STRICT_UEFI_TYPES as more of a enforce the coding standard > kind of thing? Hmmm, okay. That makes sense. The macro could be advertised as, "this will give your project / platform some extra safety, but it will place coding style requirements on your project / platform that go beyond, and sometimes conflict (in case of semantically bogus code), with the UEFI spec". Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47494): https://edk2.groups.io/g/devel/message/47494 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
On 09/18/19 17:16, Kinney, Michael D wrote: >> -Original Message- >> From: Laszlo Ersek >> However, if we wanted to allow new projects to #define >> STRICTER_UEFI_TYPES as their normal mode of operation >> (and not just for a sanity check in CI), then we'd have >> to update the UEFI spec too. >> >> Otherwise, code that is technically spec-conformant >> (albeit semantically nonsensical), like I mentioned up- >> thread, would no longer compile: >> >> EFI_HANDLE Foobar; >> UINT64 Val; >> >> Foobar = > > Does this example build without warnings on all compilers. I can't test "all" compilers :), but yes, per the C standard, it has to. "Foobar" is a pointer-to-void, and "" is a pointer-to-unsigned-long-long. Such an assignment satisfies the following passages in C99: 6. Language 6.3 Conversions 6.3.2 Other operands 6.3.2.3 Pointers 1 A pointer to void may be converted to or from a pointer to any incomplete or object type. A pointer to any incomplete or object type may be converted to a pointer to void and back again; the result shall compare equal to the original pointer. 6.5 Expressions 6.5.4 Cast operators Constraints 3 Conversions that involve pointers, other than where permitted by the constraints of 6.5.16.1, shall be specified by means of an explicit cast. 6.5.16 Assignment operators 6.5.16.1 Simple assignment Constraints 1 One of the following shall hold: [...] - one operand is a pointer to an object or incomplete type and the other is a pointer to a qualified or unqualified version of void, and the type pointed to by the left has all the qualifiers of the type pointed to by the right; [...] > I thought we usually have to add some typecasts: > >Foobar = (EFI_HANDLE) That's exactly the problem with EFI_HANDLE being a typedef to (void*) -- the explicit cast is not required. Note the "other than" language in 6.5.4 paragraph 3. > Or > >Foobar = (EFI_HANDLE)(UINTN) > > For examples like this, adding an explicit typecast would be an > improvement. So finding and reviewing and fixing these would be > a good improvement. The problem is that the Foobar = assignment is technically valid, considering both the C standard and the UEFI spec. Breaking it would be a semantic improvement, but still in conflict with what UEFI-2.8 promises. Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47492): https://edk2.groups.io/g/devel/message/47492 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
On Wed, Sep 18, 2019 at 08:55:42AM -0700, Andrew Fish via Groups.Io wrote: > >> #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 > > > > Technically, this would work well. > > > > However, if we wanted to allow new projects to #define > > STRICTER_UEFI_TYPES as their normal mode of operation (and not just for > > a sanity check in CI), then we'd have to update the UEFI spec too. > > > > Otherwise, code that is technically spec-conformant (albeit semantically > > nonsensical), like I mentioned up-thread, would no longer compile: > > I think helping people NOT write nonsensical code is good. It is > very good idea and I'd like to add it to the spec but as you point > out it would break a lot of existing code so I'm not sure it is > possible. I guess we could try to add a strict mode to the spec but > given the types are defined in tables that may be problematic. I think adding a strict mode to the specification should be doable - an important aspect is that this should[1] only break *builds* of existing code, never the execution of existing applications/drivers on firmware built to the strict mode. (Unless I'm missing something obvious.) [1] It is always possible *some* toolchain does something weird that is already wrong but not visible, and this change would expose the underlying fault. This is not necessarily bad. The specification could then describe the problematic types within #ifdef starements. Best Regards, Leif > We have coding standards that are more strict than what the C spec > allows. So I would see the STRICT_UEFI_TYPES as more of a enforce > the coding standard kind of thing? > > Thanks, > > Andrew Fish > > > EFI_HANDLE Foobar; > > UINT64 Val; > > > > Foobar = > > > > Thanks > > Laszlo > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47489): https://edk2.groups.io/g/devel/message/47489 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
> On Sep 18, 2019, at 1:41 AM, Laszlo Ersek wrote: > > On 09/17/19 22:22, Andrew Fish wrote: >> >> >>> On Sep 17, 2019, at 1:06 PM, Ni, Ray 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 > > Technically, this would work well. > > However, if we wanted to allow new projects to #define > STRICTER_UEFI_TYPES as their normal mode of operation (and not just for > a sanity check in CI), then we'd have to update the UEFI spec too. > > Otherwise, code that is technically spec-conformant (albeit semantically > nonsensical), like I mentioned up-thread, would no longer compile: > Laszlo, I think helping people NOT write nonsensical code is good. It is very good idea and I'd like to add it to the spec but as you point out it would break a lot of existing code so I'm not sure it is possible. I guess we could try to add a strict mode to the spec but given the types are defined in tables that may be problematic. We have coding standards that are more strict than what the C spec allows. So I would see the STRICT_UEFI_TYPES as more of a enforce the coding standard kind of thing? Thanks, Andrew Fish > EFI_HANDLE Foobar; > UINT64 Val; > > Foobar = > > Thanks > Laszlo > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47485): https://edk2.groups.io/g/devel/message/47485 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
> -Original Message- > From: Laszlo Ersek > Sent: Wednesday, September 18, 2019 1:42 AM > To: Andrew Fish ; Ni, Ray > > Cc: devel@edk2.groups.io; Achin Gupta > ; Anthony Perard > ; Ard Biesheuvel > ; You, Benjamin > ; Zhang, Chao B > ; Bi, Dandan > ; David Woodhouse > ; Dong, Eric > ; Dong, Guo ; > Wu, Hao A ; Carsey, Jaben > ; Wang, Jian J > ; Wu, Jiaxin > ; Yao, Jiewen > ; Justen, Jordan L > ; Julien Grall > ; Leif Lindholm > ; Gao, Liming > ; Ma, Maurice > ; Kinney, Michael D > ; Fu, Siyuan > ; Supreeth Venkatesh > ; Gao, Zhichao > > Subject: Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: > edk2: turn standard handle types into pointers to non- > VOID > > On 09/17/19 22:22, Andrew Fish wrote: > > > > > >> On Sep 17, 2019, at 1:06 PM, Ni, Ray > 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 > > Technically, this would work well. > > However, if we wanted to allow new projects to #define > STRICTER_UEFI_TYPES as their normal mode of operation > (and not just for a sanity check in CI), then we'd have > to update the UEFI spec too. > > Otherwise, code that is technically spec-conformant > (albeit semantically nonsensical), like I mentioned up- > thread, would no longer compile: > > EFI_HANDLE Foobar; > UINT64 Val; > > Foobar = Does this example build without warnings on all compilers. I thought we usually have to add some typecasts: Foobar = (EFI_HANDLE) Or Foobar = (EFI_HANDLE)(UINTN) For examples like this, adding an explicit typecast would be an improvement. So finding and reviewing and fixing these would be a good improvement. > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47482): https://edk2.groups.io/g/devel/message/47482 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
On 09/17/19 22:22, Andrew Fish wrote: > > >> On Sep 17, 2019, at 1:06 PM, Ni, Ray 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 Technically, this would work well. However, if we wanted to allow new projects to #define STRICTER_UEFI_TYPES as their normal mode of operation (and not just for a sanity check in CI), then we'd have to update the UEFI spec too. Otherwise, code that is technically spec-conformant (albeit semantically nonsensical), like I mentioned up-thread, would no longer compile: EFI_HANDLE Foobar; UINT64 Val; Foobar = Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47469): https://edk2.groups.io/g/devel/message/47469 Mute This Topic: https://groups.io/mt/34180199/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
On 09/17/19 22:06, Ni, Ray 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.
Yes, I agree; that's quite likely.
> 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?
There are two concerns:
(1) Applying this patch would cause edk2 to diverge from the UEFI / PI /
Shell specifications.
For example, consider the following 3rd party code:
EFI_HANDLE Foobar;
UINT64 Val;
Foobar =
*Semantically*, this code is wrong. However, given the publicly
specified EFI_HANDLE typedef in the UEFI spec, the code is *technically*
correct.
If we changed the EFI_HANDLE typedef in edk2, as follows:
struct EFI_OBJECT;
typedef struct EFI_OBJECT *EFI_HANDLE;
Then the assignment to Foobar above, in the 3rd party code, would become
a C language constraint violation -- and for constraint violations, C
compilers are required to emit diagnostics.
In brief, it would build-break 3rd party code that currently conforms to
the UEFI spec.
(2) Some of the changes in the patch are not optimal with regard to
"data hiding".
An example for good data hiding is the example above: "struct
EFI_OBJECT" being an incomplete type, without any details shared in the
public header.
However, in the patch we have bad data hiding too:
- typedef LIST_ENTRY*EFI_FONT_HANDLE
- typedef UINT8 *EFI_S3_BOOT_SCRIPT_POSITION
- typedef EFI_FILE_PROTOCOL *SHELL_FILE_HANDLE;
These typedefs, while they serve the purposes of the present patch set,
expose too much information (= the actual implementation) about the types.
For solving this data hiding problem, we'd have to (a) introduce
intermittent structure types (similar to EFI_OBJECT), and then (b) we'd
have to rework the rest of the implementation for that. Using
EFI_FONT_HANDLE as example:
(a) introduce the intermittent (opaque) structure type:
struct EFI_FONT_OBJECT;
typedef struct EFI_FONT_OBJECT *EFI_FONT_HANDLE;
(b) rework the implementation:
struct EFI_FONT_OBJECT {
LIST_ENTRY Entry;
};
typedef struct _HII_GLOBAL_FONT_INFO {
UINTN Signature;
struct EFI_FONT_OBJECTFontObject;
HII_FONT_PACKAGE_INSTANCE *FontPackage;
UINTN FontInfoSize;
EFI_FONT_INFO *FontInfo;
} HII_GLOBAL_FONT_INFO;
and then every reference to "HII_GLOBAL_FONT_INFO.Entry" would have to
be updated to "HII_GLOBAL_FONT_INFO.FontObject.Entry".
It's likely doable, but it would take a lot of work.
If we wanted to continue using this patch for sanity-checking edk2, then
one option would be to bracket the changes with #ifdef's. That is, make
the "type separation" dependent on a new feature test macro.
Normally, the feature test macro would not be defined (as it breaks UEFI
spec conformance, see above). But in CI, one set of the builds would
define the macro, for sanity checking edk2 itself.
Thanks
Laszlo
>
> Thanks,
> Ray
>
>
>> -Original Message-
>> From: devel@edk2.groups.io On Behalf Of Laszlo Ersek
>> Sent: Tuesday, September 17, 2019 12:49 PM
>> To: edk2-devel-groups-io
>> Cc: Achin Gupta ; Andrew Fish ;
>> Anthony Perard ;
>> Ard Biesheuvel ; You, Benjamin
>> ; Zhang, Chao B
>> ; Bi, Dandan ; David Woodhouse
>> ; Dong, Eric
>> ; Dong, Guo ; Wu, Hao A
>> ; Carsey, Jaben
>> ; Wang, Jian J ; Wu, Jiaxin
>> ; Yao, Jiewen
>> ; Justen, Jordan L ; Julien
>> Grall ; Leif Lindholm
>> ; Gao, Liming ; Ma, Maurice
>> ; Kinney, Michael
>> D ; Ni, Ray ; Fu, Siyuan
>> ; Supreeth Venkatesh
>> ; Gao, Zhichao
>> 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
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
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 On > Behalf Of Andrew Fish via Groups.Io > Sent: Tuesday, September 17, 2019 2:07 PM > To: devel@edk2.groups.io; Ni, Ray > Cc: ler...@redhat.com; Achin Gupta > ; Anthony Perard > ; Ard Biesheuvel > ; You, Benjamin > ; Zhang, Chao B > ; Bi, Dandan > ; David Woodhouse > ; Dong, Eric > ; Dong, Guo ; > Wu, Hao A ; Carsey, Jaben > ; Wang, Jian J > ; Wu, Jiaxin > ; Yao, Jiewen > ; Justen, Jordan L > ; Julien Grall > ; Leif Lindholm > ; Gao, Liming > ; Ma, Maurice > ; Kinney, Michael D > ; Fu, Siyuan > ; Supreeth Venkatesh > ; Gao, Zhichao > > 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 > 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 On > Behalf Of Andrew > >> Fish via Groups.Io > >> Sent: Tuesday, September 17, 2019 1:22 PM > >> To: Ni, Ray > >> Cc: devel@edk2.groups.io; ler...@redhat.com; Achin > Gupta > >> ; Anthony Perard > ; > >> Ard Biesheuvel ; You, > Benjamin > >> ; Zhang, Chao B > ; Bi, > >> Dandan ; David Woodhouse > ; > >> Dong, Eric ; Dong, Guo > ; Wu, > >> Hao A ; Carsey, Jaben > ; > >> Wang, Jian J ; Wu, Jiaxin > >> ; Yao, Jiewen > ; Justen, > >> Jordan L ; Julien Grall > >> ; Leif Lindholm > ; > >> Gao, Liming ; Ma, Maurice > >> ; Kinney, Michael D > >> ; Fu, Siyuan > ; > >> Supreeth Venkatesh ; > Gao, Zhichao > >> > >> 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 > 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 > On Behalf Of > >>>> Laszlo Ersek > >>>> Sent: Tuesday, September 17, 2019 12:49 PM > >>>> To: edk2-devel-groups-io > >>>> Cc: Achin Gupta ; Andrew Fish > >>>> ; Anthony Perard > >> ; > >>>> Ard Biesheuvel ; You, > Benjamin > >>>> ; Zhang, Chao B > ; > >>>> Bi, Dandan ; David Woodhouse > >>>> ; Dong, > >> Eric > >>>> ; Dong, Guo > ; Wu, Hao A > >>>> ; Carsey, Jaben > ; Wang, > >>>> Jian J ; Wu, Jiaxin > ; > >>>> Yao, Jiewen ; Justen, Jordan > L > >>>> ; Julien Grall > ; > >>>> Leif > >> Lindholm > >>>> ; Gao, Liming > ; Ma, >
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 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 On Behalf Of Andrew Fish >> via Groups.Io >> Sent: Tuesday, September 17, 2019 1:22 PM >> To: Ni, Ray >> Cc: devel@edk2.groups.io; ler...@redhat.com; Achin Gupta >> ; Anthony Perard >> ; Ard Biesheuvel ; >> You, Benjamin ; >> Zhang, Chao B ; Bi, Dandan ; >> David Woodhouse >> ; Dong, Eric ; Dong, Guo >> ; Wu, Hao A >> ; Carsey, Jaben ; Wang, Jian J >> ; Wu, Jiaxin >> ; Yao, Jiewen ; Justen, Jordan L >> ; Julien Grall >> ; Leif Lindholm ; Gao, >> Liming ; Ma, Maurice >> ; Kinney, Michael D ; Fu, >> Siyuan ; Supreeth >> Venkatesh ; Gao, Zhichao >> 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 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 On Behalf Of Laszlo Ersek >>>> Sent: Tuesday, September 17, 2019 12:49 PM >>>> To: edk2-devel-groups-io >>>> Cc: Achin Gupta ; Andrew Fish ; >>>> Anthony Perard >> ; >>>> Ard Biesheuvel ; You, Benjamin >>>> ; Zhang, Chao B >>>> ; Bi, Dandan ; David >>>> Woodhouse ; Dong, >> Eric >>>> ; Dong, Guo ; Wu, Hao A >>>> ; Carsey, Jaben >>>> ; Wang, Jian J ; Wu, Jiaxin >>>> ; Yao, Jiewen >>>> ; Justen, Jordan L ; >>>> Julien Grall ; Leif >> Lindholm >>>> ; Gao, Liming ; Ma, >>>> Maurice ; Kinney, >> Michael >>>> D ; Ni, Ray ; Fu, Siyuan >>>> ; Supreeth Venkatesh >>>> ; Gao, Zhichao >>>> 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, >>&g
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
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 On Behalf Of Andrew Fish > via Groups.Io > Sent: Tuesday, September 17, 2019 1:22 PM > To: Ni, Ray > Cc: devel@edk2.groups.io; ler...@redhat.com; Achin Gupta > ; Anthony Perard > ; Ard Biesheuvel ; You, > Benjamin ; > Zhang, Chao B ; Bi, Dandan ; > David Woodhouse > ; Dong, Eric ; Dong, Guo > ; Wu, Hao A > ; Carsey, Jaben ; Wang, Jian J > ; Wu, Jiaxin > ; Yao, Jiewen ; Justen, Jordan L > ; Julien Grall > ; Leif Lindholm ; Gao, Liming > ; Ma, Maurice > ; Kinney, Michael D ; Fu, > Siyuan ; Supreeth > Venkatesh ; Gao, Zhichao > 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 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 On Behalf Of Laszlo Ersek > >> Sent: Tuesday, September 17, 2019 12:49 PM > >> To: edk2-devel-groups-io > >> Cc: Achin Gupta ; Andrew Fish ; > >> Anthony Perard > ; > >> Ard Biesheuvel ; You, Benjamin > >> ; Zhang, Chao B > >> ; Bi, Dandan ; David > >> Woodhouse ; Dong, > Eric > >> ; Dong, Guo ; Wu, Hao A > >> ; Carsey, Jaben > >> ; Wang, Jian J ; Wu, Jiaxin > >> ; Yao, Jiewen > >> ; Justen, Jordan L ; > >> Julien Grall ; Leif > Lindholm > >> ; Gao, Liming ; Ma, > >> Maurice ; Kinney, > Michael > >> D ; Ni, Ray ; Fu, Siyuan > >> ; Supreeth Venkatesh > >> ; Gao, Zhichao > >> 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 incompatib
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 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 On Behalf Of Laszlo Ersek >> Sent: Tuesday, September 17, 2019 12:49 PM >> To: edk2-devel-groups-io >> Cc: Achin Gupta ; Andrew Fish ; >> Anthony Perard ; >> Ard Biesheuvel ; You, Benjamin >> ; Zhang, Chao B >> ; Bi, Dandan ; David Woodhouse >> ; Dong, Eric >> ; Dong, Guo ; Wu, Hao A >> ; Carsey, Jaben >> ; Wang, Jian J ; Wu, Jiaxin >> ; Yao, Jiewen >> ; Justen, Jordan L ; Julien >> Grall ; Leif Lindholm >> ; Gao, Liming ; Ma, Maurice >> ; Kinney, Michael >> D ; Ni, Ray ; Fu, Siyuan >> ; Supreeth Venkatesh >> ; Gao, Zhichao >> 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 >> Cc: Andrew Fish >> Cc: Anthony Perard >> Cc: Ard Biesheuvel >> Cc: Benjamin You >> Cc: Chao Zhang >> Cc: Dandan Bi >> Cc: David Woodhouse >> Cc: Eric Dong >> Cc: Guo Dong >> Cc: Hao A Wu >> Cc: Jaben Carsey >> Cc: Jian J Wang >> Cc: Jian Wang >> Cc: Jiaxin Wu >> Cc: Jiewen Yao >> Cc: Jordan Justen >> Cc: Julien Grall >> Cc: Leif Lindholm >> Cc: Liming Gao >> Cc: Maurice Ma >> Cc: Michael D Kinney >> Cc: Ray Ni >> Cc: Siyuan Fu >> Cc: Supreeth Venkatesh >> Cc: Zhichao Gao >> Signed-off-by: Laszlo Ersek >> --- >> MdePkg/Include/Pi/PiPeiCis.h | 6 -- >> MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++- >> MdePkg/Include/Protocol/Bis
Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID
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 On Behalf Of Laszlo Ersek > Sent: Tuesday, September 17, 2019 12:49 PM > To: edk2-devel-groups-io > Cc: Achin Gupta ; Andrew Fish ; Anthony > Perard ; > Ard Biesheuvel ; You, Benjamin > ; Zhang, Chao B > ; Bi, Dandan ; David Woodhouse > ; Dong, Eric > ; Dong, Guo ; Wu, Hao A > ; Carsey, Jaben > ; Wang, Jian J ; Wu, Jiaxin > ; Yao, Jiewen > ; Justen, Jordan L ; Julien > Grall ; Leif Lindholm > ; Gao, Liming ; Ma, Maurice > ; Kinney, Michael > D ; Ni, Ray ; Fu, Siyuan > ; Supreeth Venkatesh > ; Gao, Zhichao > 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 > Cc: Andrew Fish > Cc: Anthony Perard > Cc: Ard Biesheuvel > Cc: Benjamin You > Cc: Chao Zhang > Cc: Dandan Bi > Cc: David Woodhouse > Cc: Eric Dong > Cc: Guo Dong > Cc: Hao A Wu > Cc: Jaben Carsey > Cc: Jian J Wang > Cc: Jian Wang > Cc: Jiaxin Wu > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Julien Grall > Cc: Leif Lindholm > Cc: Liming Gao > Cc: Maurice Ma > Cc: Michael D Kinney > Cc: Ray Ni > Cc: Siyuan Fu > Cc: Supreeth Venkatesh > Cc: Zhichao Gao > Signed-off-by: Laszlo Ersek > --- > MdePkg/Include/Pi/PiPeiCis.h | 6 -- > MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++- > MdePkg/Include/Protocol/Bis.h| 3 ++- > MdePkg/Include/Protocol/Eap.h| 3 ++- > MdePkg/Include/Protocol/HiiFont.h| 3 +-- > MdePkg/Include/Protocol/MmMp.h | 3 ++- > MdePkg/Include/Protocol/S3SaveState.h| 2 +- > MdePkg/Include/Protocol/Shell.h | 3 ++- > MdePkg/Include/Protocol/UserManager.h| 9 ++--- > MdePkg/Include/Uefi/UefiBaseType.h | 6 -- > MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++- > MdeModulePkg/Core/Dxe/Event/Event.h | 2 +- > MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +- > MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +- > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +- > MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +- > StandaloneMmPkg/Co
[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
Cc: Andrew Fish
Cc: Anthony Perard
Cc: Ard Biesheuvel
Cc: Benjamin You
Cc: Chao Zhang
Cc: Dandan Bi
Cc: David Woodhouse
Cc: Eric Dong
Cc: Guo Dong
Cc: Hao A Wu
Cc: Jaben Carsey
Cc: Jian J Wang
Cc: Jian Wang
Cc: Jiaxin Wu
Cc: Jiewen Yao
Cc: Jordan Justen
Cc: Julien Grall
Cc: Leif Lindholm
Cc: Liming Gao
Cc: Maurice Ma
Cc: Michael D Kinney
Cc: Ray Ni
Cc: Siyuan Fu
Cc: Supreeth Venkatesh
Cc: Zhichao Gao
Signed-off-by: Laszlo Ersek
---
MdePkg/Include/Pi/PiPeiCis.h | 6 --
MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h | 3 ++-
MdePkg/Include/Protocol/Bis.h| 3 ++-
MdePkg/Include/Protocol/Eap.h| 3 ++-
MdePkg/Include/Protocol/HiiFont.h| 3 +--
MdePkg/Include/Protocol/MmMp.h | 3 ++-
MdePkg/Include/Protocol/S3SaveState.h| 2 +-
MdePkg/Include/Protocol/Shell.h | 3 ++-
MdePkg/Include/Protocol/UserManager.h| 9 ++---
MdePkg/Include/Uefi/UefiBaseType.h | 6 --
MdePkg/Include/Uefi/UefiInternalFormRepresentation.h | 3 ++-
MdeModulePkg/Core/Dxe/Event/Event.h | 2 +-
MdeModulePkg/Core/Dxe/Hand/Handle.h | 2 +-
MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 2 +-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h | 2 +-
MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h | 2 +-
StandaloneMmPkg/Core/StandaloneMmCore.h | 2 +-
17 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/MdePkg/Include/Pi/PiPeiCis.h b/MdePkg/Include/Pi/PiPeiCis.h
index d9d4ed7d413a..3e9e82b62ae9 100644
--- a/MdePkg/Include/Pi/PiPeiCis.h
+++ b/MdePkg/Include/Pi/PiPeiCis.h
@@ -18,12 +18,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
///
/// The handles of EFI FV.
///
-typedef VOID*EFI_PEI_FV_HANDLE;
+struct EFI_PEI_FV_OBJECT;
+typedef struct EFI_PEI_FV_OBJECT *EFI_PEI_FV_HANDLE;
///
/// 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
