[edk2-devel] [PATCH 00/35] edk2: clean up the usage of standardized (VOID*) typedefs

Wu, Hao A hao.a.wu at intel.com
Thu Sep 19 00:32:36 UTC 2019


> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, September 18, 2019 3:49 AM
> 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 00/35] edk2: clean up the usage of
> standardized (VOID*) typedefs
> 
> Repository: https://github.com/lersek/edk2.git
> Branch:     voidptr
> 
> The UEFI / PI / Shell specifications define a number of standard types
> as pointers to VOID. This is arguably a design mistake; those types
> should have been pointers to distinct incomplete union or structure
> types. Here's why:
> 
> Roughly paraphrasing the constraints from ISO C99 "6.5.16.1 Simple
> assignment" and "6.5.4 Cast operators", any pointer-to-object type
> converts implicitly to, and from, pointer-to-void, provided const /
> volatile qualifications are not relaxed. Such implicit conversions
> prevent compilers from catching at least the following two kinds of
> coding mistakes:
> 
> - mixing up one 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).
> 
> This series first separates these standard types from each other, in the
> first patch, which is *not* being proposed for merging. This unmasks a
> number of warts (semantic issues, or actual bugs) in the source code, in
> the form of build breakages. The rest of the series works through those
> breakages, cleaning and fixing the code.
> 
> Every DSC file in the edk2 tree was built for at least one of the NOOPT,
> DEBUG, RELEASE targets (NOOPT being preferred), with the GCC48 toolchain
> (for IA32 / X64) and the GCC5 toolchain (for ARM / AARCH64). Of course,
> the build arches were restricted to the SUPPORTED_ARCHITECTURES stated
> in the individual DSC files.
> 
> There were two exceptions to the above rule: DynamicTablesPkg was only
> build-tested with AARCH64 (despite its SUPPORTED_ARCHITECTURES), given
> that 32-bit ARM has no ACPI bindings. StandaloneMmPkg too was only
> build-tested with AARCH64; it doesn't actually support IA32/X64 yet.
> 
> Regarding boot & runtime tests, ArmVirtQemu on AARCH64 was tested with
> booting to the OS (RHEL7). Furthermore, I exercised OVMF with my usual
> boot and S3 tests, covering IA32, IA32X64, and X64. Finally, some other
> individual tests (noted per patch) were done with OVMF.
> 
> Cc: Achin Gupta <achin.gupta at arm.com>
> Cc: Andrew Fish <afish at apple.com>
> Cc: Anthony Perard <anthony.perard at citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> Cc: Benjamin You <benjamin.you at intel.com>
> Cc: Chao Zhang <chao.b.zhang at intel.com>
> Cc: Dandan Bi <dandan.bi at intel.com>
> Cc: David Woodhouse <dwmw2 at infradead.org>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Guo Dong <guo.dong at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Jaben Carsey <jaben.carsey at intel.com>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Jian Wang <jian.j.wang at intel.com>
> Cc: Jiaxin Wu <jiaxin.wu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Julien Grall <julien.grall at arm.com>
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Maurice Ma <maurice.ma at intel.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Siyuan Fu <siyuan.fu at intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>
> Cc: Zhichao Gao <zhichao.gao at intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (35):
>   DO NOT APPLY: edk2: turn standard handle types into pointers to
>     non-VOID
>   EmbeddedPkg: add missing EFIAPI calling convention specifiers
>   EmbeddedPkg/AndroidFastbootTransportTcpDxe: fix DestroyChild() call
>   EmbeddedPkg/Universal/MmcDxe: "fix" CloseProtocol() call in
>     BindingStop()
>   EmulatorPkg/DxeTimerLib: drop superfluous cast
>   EmulatorPkg: stop abusing EFI_HANDLE for keystroke notify registration
>   MdeModulePkg: fix cast in GetModuleInfoFromHandle() calls
>   MdeModulePkg/UefiHiiLib: stop using EFI_HANDLE in place of
>     EFI_HII_HANDLE
>   MdeModulePkg: stop abusing EFI_EVENT for protocol notify registration
>   MdeModulePkg/PlatformVarCleanupLib: fix HiiConstructConfigHdr() call
>   MdeModulePkg: document workaround for EFI_RUNTIME_EVENT_ENTRY PI
> spec
>     bug
>   MdeModulePkg: stop abusing EFI_HANDLE for keystroke notify
>     registration
>   MdeModulePkg: PEI Core: clean up "AprioriFile" handling in
>     FindFileEx()
>   MdeModulePkg: fix UninstallMultipleProtocolInterfaces() calls
>   MdeModulePkg/PiSmmCore: make type punning consistent
>   MdeModulePkg/S3SaveState: cast Position for S3BootScriptLib explicitly


For the patches made to MdeModulePkg (patch 7~16),
Reviewed-by: Hao A Wu <hao.a.wu at intel.com>

Best Regards,
Hao Wu


>   MdePkg/DxeServicesLib: remove bogus cast
>   NetworkPkg/DxeNetLib: fix type typo in NetLibGetMacAddress()
>   NetworkPkg: fix CloseProtocol & UninstallMultipleProtocolInterfaces
>     calls
>   NetworkPkg/Ip4Dxe: fix NetLibDestroyServiceChild() call
>   NetworkPkg/TcpDxe: fix SockFreeFoo() parameter list
>   OvmfPkg/XenBusDxe: fix UninstallMultipleProtocolInterfaces() call
>   OvmfPkg/VirtioNetDxe: fix SignalEvent() call
>   OvmfPkg/PlatformDxe: fix EFI_HII_HANDLE parameters of internal
>     functions
>   OvmfPkg/VideoDxe: document EFI_EDID_OVERRIDE_PROTOCOL.GetEdid()
> call
>   SecurityPkg: fix UninstallMultipleProtocolInterfaces() calls
>   SecurityPkg: stop abusing EFI_EVENT for protocol notify registration
>   ShellPkg/UefiShellDriver1CommandsLib: fix parameter list typo
>   ShellPkg: stop using EFI_HANDLE in place of EFI_HII_HANDLE
>   ShellPkg: stop taking EFI_HANDLE in place of SHELL_FILE_HANDLE
>   ShellPkg/UefiShellDebug1CommandsLib: fix ShellCloseFile() call
>   ShellPkg/UefiShellLib: clarify workaround for unfixable EdkShell bug
>   StandaloneMmPkg/Core: stop abusing EFI_HANDLE for FwVolHeader
> tracking
>   UefiPayloadPkg/BlSupportPei: fix MMCONFIG assignment from XSDT
>   UefiPayloadPkg/BlSupportDxe: fix ReserveResourceInGcd() calls
> 
> 
> EmbeddedPkg/Drivers/AndroidFastbootTransportTcpDxe/FastbootTranspor
> tTcp.c      |  2 +-
>  EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.c                            |  1
> +
>  EmbeddedPkg/Drivers/Lan9118Dxe/Lan9118Dxe.c                                    |  1 +
>  EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.h                               | 32
> ++++++--
>  EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132AtaPassThru.c                        |  8
> ++
>  EmbeddedPkg/GdbStub/GdbStubInternal.h                                          |  9 +++
>  EmbeddedPkg/MetronomeDxe/Metronome.c                                           |  1 +
>  EmbeddedPkg/Universal/MmcDxe/Mmc.c                                             |  5 +-
>  EmulatorPkg/EmuGopDxe/GopInput.c                                               |  4 +-
>  EmulatorPkg/Library/DxeTimerLib/DxeTimerLib.c                                  |  2 +-
>  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 +-
>  MdeModulePkg/Core/Dxe/Event/Event.c                                            |  8 ++
>  MdeModulePkg/Core/Dxe/Event/Event.h                                            |  2 +-
>  MdeModulePkg/Core/Dxe/Hand/Handle.h                                            |  2 +-
>  MdeModulePkg/Core/Pei/FwVol/FwVol.c                                            |  2 +-
>  MdeModulePkg/Core/Pei/FwVol/FwVol.h                                            |  2 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h                                        |  2 +-
>  MdeModulePkg/Core/PiSmmCore/Smi.c                                              |  8 +-
>  MdeModulePkg/Core/RuntimeDxe/Runtime.c                                         | 10 ++-
> 
> MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> | 12 +--
>  MdeModulePkg/Library/PlatformVarCleanupLib/PlatVarCleanupLib.c
> |  6 +-
> 
> MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePerformanceLi
> b.c             |  8 +-
>  MdeModulePkg/Library/UefiHiiLib/HiiString.c                                    |  4 +-
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h                             |  2
> +-
>  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveState.c                       |
> 4 +-
>  MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.c
> |  4 +-
>  MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitter.c
> |  2 +-
>  MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c                          |
> 4 +-
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.c
> |  2 +-
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.
> c           |  2 +-
>  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabase.h                            |  2
> +-
>  MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c                           |  2
> +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> |  2 +-
>  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
> +-
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.c                                 |  2 +-
>  NetworkPkg/DnsDxe/DnsDriver.c                                                  |  4 +-
>  NetworkPkg/IScsiDxe/IScsiConfig.c                                              |  2 +-
>  NetworkPkg/Ip4Dxe/Ip4Driver.c                                                  |  2 +-
>  NetworkPkg/Ip4Dxe/Ip4If.c                                                      |  4 +-
>  NetworkPkg/Ip6Dxe/Ip6Driver.c                                                  |  2 +-
>  NetworkPkg/Library/DxeNetLib/DxeNetLib.c                                       |  2 +-
>  NetworkPkg/Mtftp4Dxe/Mtftp4Driver.c                                            |  2 +-
>  NetworkPkg/TcpDxe/SockImpl.c                                                   |  4 +-
>  NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.c                                 |  2 +-
>  OvmfPkg/Csm/BiosThunk/VideoDxe/BiosVideo.c                                     |  6 +-
>  OvmfPkg/PlatformDxe/Platform.c                                                 |  4 +-
>  OvmfPkg/VirtioNetDxe/Events.c                                                  |  2 +-
>  OvmfPkg/XenBusDxe/XenBus.c                                                     |  2 +-
>  SecurityPkg/HddPassword/HddPasswordDxe.c                                       |  2 +-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c                                  |  2 +-
>  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDriver.c                                 |  2 +-
> 
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
> gDriver.c |  2 +-
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c                                  |
> 6 +-
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h                                  |
> 4 +-
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c                              |
> 6 +-
>  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.h                              |
> 4 +-
>  ShellPkg/Include/Library/ShellCommandLib.h                                     |  2 +-
>  ShellPkg/Include/Library/ShellLib.h                                            |  4 +-
>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c                   |  2
> +-
>  ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.c
> |  2 +-
>  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.c
> |  2 +-
>  ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.h
> |  2 +-
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c                  |
> 2 +-
> 
> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.c       |  2 +-
> 
> ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Comman
> dsLib.h       |  2 +-
>  ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c                         |  4
> +-
> 
> ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Command
> sLib.c     |  2 +-
> 
> ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Command
> sLib.h     |  2 +-
> 
> ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsL
> ib.c       |  2 +-
> 
> ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsL
> ib.h       |  2 +-
>  ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c                               |  2 +-
> 
> ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsL
> ib.c       |  2 +-
> 
> ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsL
> ib.h       |  2 +-
>  ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c                            |  2 +-
> 
> ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsL
> ib.c       |  2 +-
> 
> ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsL
> ib.h       |  2 +-
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c                                   | 26 ++++++-
> 
> ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.c   |  2 +-
> 
> ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Com
> mandsLib.h   |  2 +-
> 
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
> mandsLib.c   |  2 +-
> 
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Com
> mandsLib.h   |  2 +-
>  StandaloneMmPkg/Core/Dispatcher.c                                              | 80
> +++++++++++---------
>  StandaloneMmPkg/Core/FwVol.c                                                   | 16 ++--
>  StandaloneMmPkg/Core/StandaloneMmCore.h                                        |  4 +-
>  UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c                                     |  4 +-
>  UefiPayloadPkg/BlSupportPei/BlSupportPei.c                                     | 19 +++--
>  102 files changed, 294 insertions(+), 194 deletions(-)
> 
> --
> 2.19.1.3.g30247aa5d201
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> 
> View/Reply Online (#47387):
> https://edk2.groups.io/g/devel/message/47387
> Mute This Topic: https://groups.io/mt/34180197/1768737
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [hao.a.wu at intel.com]
> -=-=-=-=-=-=


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

View/Reply Online (#47514): https://edk2.groups.io/g/devel/message/47514
Mute This Topic: https://groups.io/mt/34180197/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list