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.3376.1570614655554875086 for ; Wed, 09 Oct 2019 02:50:55 -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-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D27A4C057F31; Wed, 9 Oct 2019 09:50:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-11.rdu2.redhat.com [10.10.120.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 48654100194E; Wed, 9 Oct 2019 09:50:49 +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> <70182821-2cb0-0384-216d-85f0783d6858@redhat.com> Message-ID: <938428d0-cae5-6492-e80e-b21df7ef72f6@redhat.com> Date: Wed, 9 Oct 2019 11:50:48 +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: <70182821-2cb0-0384-216d-85f0783d6858@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Wed, 09 Oct 2019 09:50:55 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > , 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. Commit range 2de1f611be06..976d0353a6ce. Thanks, Laszlo