On 10/09/19 01:49, Laszlo Ersek wrote: > On 09/17/19 21:49, Laszlo Ersek wrote: >> 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. > > This patch series is now ready to be pushed (it's fully reviewed), > except for the following two patches: > > * [edk2-devel] [PATCH 01/35] > DO NOT APPLY: edk2: turn standard handle types into pointers to > non-VOID > > * [edk2-devel] [PATCH 25/35] > OvmfPkg/VideoDxe: document EFI_EDID_OVERRIDE_PROTOCOL.GetEdid() call > > Regarding 01/35, it was never meant to be pushed (certainly not in this > form); plus it would likely be too early for a number of out-of-tree > platforms. > > We have discussed enabling "strict UEFI types" (even by default, > possibly). That's a great goal. And I don't have any time for pursuing > it. :( So yes, I'm aware the problems will likely reproduce over time; > I'm sorry about that. > > Regarding 25/35, the original (unpatched) code might actually prove > correct there (needing a fix in the EFI_EDID_OVERRIDE_PROTOCOL_GET_EDID > definition instead). That depends on > <https://mantis.uefi.org/mantis/view.php?id=2018>, however. > > I've reached out to Andy Hayes here: > > 985de369-7880-b6cc-46e7-5a2edca6582b@redhat.com">http://mid.mail-archive.com/985de369-7880-b6cc-46e7-5a2edca6582b@redhat.com > https://edk2.groups.io/g/devel/message/48487 > > but I've not received any feedback yet. > > So tomorrow I plan to re-run some sanity builds, with all patches except > #01 and #25 applied. If the builds still complete, I'm going to push the > other 33 patches.
Commit range 2de1f611be06..976d0353a6ce. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48634): https://edk2.groups.io/g/devel/message/48634 Mute This Topic: https://groups.io/mt/34180197/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
