From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.groups.io with SMTP id smtpd.web10.49.1570578556950044171 for ; Tue, 08 Oct 2019 16:49:17 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3E98A806A75; Tue, 8 Oct 2019 23:49:16 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-109.rdu2.redhat.com [10.10.120.109]) by smtp.corp.redhat.com (Postfix) with ESMTP id BE6BF5D70D; Tue, 8 Oct 2019 23:49:10 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 00/35] edk2: clean up the usage of standardized (VOID*) typedefs From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Achin Gupta , Andrew Fish , Anthony Perard , Ard Biesheuvel , Benjamin You , Chao Zhang , Dandan Bi , David Woodhouse , Eric Dong , Guo Dong , Hao A Wu , Jaben Carsey , Jian J Wang , Jiaxin Wu , Jiewen Yao , Jordan Justen , Julien Grall , Leif Lindholm , Liming Gao , Maurice Ma , Michael D Kinney , Ray Ni , Siyuan Fu , Supreeth Venkatesh , Zhichao Gao Reply-To: devel@edk2.groups.io, lersek@redhat.com References: <20190917194935.24322-1-lersek@redhat.com> Message-ID: <70182821-2cb0-0384-216d-85f0783d6858@redhat.com> Date: Wed, 9 Oct 2019 01:49:09 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190917194935.24322-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.67]); Tue, 08 Oct 2019 23:49:16 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 , however. I've reached out to Andy Hayes here: 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. Thanks, Laszlo