From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Wed, 18 Sep 2019 10:45:20 -0700 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 6AB9D3001FEB; Wed, 18 Sep 2019 17:45:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-125-22.rdu2.redhat.com [10.10.125.22]) by smtp.corp.redhat.com (Postfix) with ESMTP id 396C95D6A5; Wed, 18 Sep 2019 17:45:14 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 01/35] DO NOT APPLY: edk2: turn standard handle types into pointers to non-VOID To: Andrew Fish , devel@edk2.groups.io Cc: "Ni, Ray" , 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" , Jordan Justen , Julien Grall , Leif Lindholm , "Gao, Liming" , "Ma, Maurice" , Mike Kinney , "Fu, Siyuan" , Supreeth Venkatesh , "Gao, Zhichao" References: <20190917194935.24322-1-lersek@redhat.com> <20190917194935.24322-2-lersek@redhat.com> <734D49CCEBEEF84792F5B80ED585239D5C2E3BE1@SHSMSX104.ccr.corp.intel.com> <1FFFA19B-454C-4B46-9DD0-339BB8011838@apple.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 18 Sep 2019 19:45:13 +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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.45]); Wed, 18 Sep 2019 17:45:19 +0000 (UTC) Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit 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