From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C1F64211B85C2 for ; Wed, 23 Jan 2019 10:27:02 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 23 Jan 2019 10:27:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,512,1539673200"; d="scan'208";a="138140506" Received: from orsmsx103.amr.corp.intel.com ([10.22.225.130]) by fmsmga004.fm.intel.com with ESMTP; 23 Jan 2019 10:27:00 -0800 Received: from orsmsx152.amr.corp.intel.com (10.22.226.39) by ORSMSX103.amr.corp.intel.com (10.22.225.130) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 23 Jan 2019 10:27:01 -0800 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.240]) by ORSMSX152.amr.corp.intel.com ([169.254.8.53]) with mapi id 14.03.0415.000; Wed, 23 Jan 2019 10:27:00 -0800 From: "Kinney, Michael D" To: "afish@apple.com" , "Kinney, Michael D" CC: Felix Poludov , "edk2-devel@lists.01.org" Thread-Topic: [edk2] History question about Base.h and its alternate parallel name space.... Should we change it? Thread-Index: AQHUrd6ShywsZwEiGEavur1WJbzcbqWyZKPwgACYcICAAA57gIAA2A7ggAHnqoCAAARoAIAFydjQgAD+yICAAJ3EUA== Date: Wed, 23 Jan 2019 18:27:00 +0000 Message-ID: References: <37D3156D-0434-4A64-BF0C-9883A4B88838@apple.com> <9333E191E0D52B4999CE63A99BA663A00302C56A66@atlms1.us.megatrends.com> <9333E191E0D52B4999CE63A99BA663A00302C573F1@atlms1.us.megatrends.com> <6388D0B3-9465-40E6-8CDF-11001D321637@apple.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: History question about Base.h and its alternate parallel name space.... Should we change it? X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Jan 2019 18:27:02 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Andrew, I think it makes sense for new code to use the UEFI type names and for existing code to remain unchanged. >>From looking at UefiBaseTypes.h, I do not think we should move all types from that file into Base.h. If we really wanted to do that, we could simple add #include of=20 UefiBaseTypes.h to Base.h. Moving a few lines from UefiBaseTypes.h to Base.h at the bottom of Base.h with a comment block that explains why the UEFI specific types/defines are available from Base.h would be another way to add a subset of types/defines. =20 That way, if we decide to move more in the future, there is a place to move them that is easy to review. Do you want to propose the specific types/defines that=20 should be moved? Best regards, Mike > -----Original Message----- > From: afish@apple.com [mailto:afish@apple.com] > Sent: Tuesday, January 22, 2019 5:00 PM > To: Kinney, Michael D > Cc: Felix Poludov ; edk2- > devel@lists.01.org > Subject: Re: [edk2] History question about Base.h and > its alternate parallel name space.... Should we change > it? >=20 >=20 >=20 > > On Jan 22, 2019, at 9:49 AM, Kinney, Michael D > wrote: > > > > Andrew, > > > > If we move all of those, then what are the code review > rules for a lib of type BASE? > > > > Is there a preference to use the current BASE types? > Under what conditions are EFI type names allowed? > > > > Is the a preference to stop using the current BASE > types all together? > > >=20 > Mike, >=20 > it seems messy to add types to Base.h to support Base > Libs. So for example IPv4_ADDRESS and IPv6_ADDRESS got > sucked into Base.h since an MdePkg Base Lib got added > that used it. That does not really seem to scale to Base > Libs in other packages. Thus I guess the preference > would be to use the common EFI_* names. I'd not advocate > removing the old names as that is going to break other > folks Base LIbs. We could do the cleanup in TianoCore. > If we want to deprecate the old names form, I guess that > could be an ifdef to allow some depreciation in the > future? >=20 > Thanks, >=20 > Andrew Fish >=20 > > Thanks, > > > > Mike > > > >> -----Original Message----- > >> From: afish@apple.com [mailto:afish@apple.com] > >> Sent: Friday, January 18, 2019 9:24 AM > >> To: Felix Poludov > >> Cc: Kinney, Michael D ; > >> edk2-devel@lists.01.org > >> Subject: Re: [edk2] History question about Base.h and > >> its alternate parallel name space.... Should we > change > >> it? > >> > >> > >> > >>> On Jan 18, 2019, at 9:08 AM, Felix Polyudov > >> wrote: > >>> > >>> Mike, > >>> > >>> I think EFI_GUID and EFI_STATUS should cover most of > >> the use cases. > >>> > >> > >> I think I missed a couple in my original mail.... > >> > >> But here are the typedef and #define names that get > >> remapped (or redone) in > >> MdePkg/Include/Uefi/UefiBaseType.h > >> > >> typedef struct { > >> UINT32 Data1; > >> UINT16 Data2; > >> UINT16 Data3; > >> UINT8 Data4[8]; > >> } GUID; > >> > >> typedef struct { > >> UINT8 Addr[4]; > >> } IPv4_ADDRESS; > >> > >> typedef struct { > >> UINT8 Addr[16]; > >> } IPv6_ADDRESS; > >> > >> typedef UINT64 PHYSICAL_ADDRESS; > >> typedef UINTN RETURN_STATUS; > >> > >> #define ENCODE_ERROR(StatusCode) > >> ((RETURN_STATUS)(MAX_BIT | (StatusCode))) > >> #define ENCODE_WARNING(StatusCode) > >> ((RETURN_STATUS)(StatusCode)) > >> #define RETURN_ERROR(StatusCode) > >> (((INTN)(RETURN_STATUS)(StatusCode)) < 0) > >> #define RETURN_SUCCESS 0 > >> #define RETURN_LOAD_ERROR ENCODE_ERROR (1) > >> #define RETURN_INVALID_PARAMETER ENCODE_ERROR (2) > >> .... > >> > >> > >> I think the cleanup would be as simple as moving > things > >> from MdePkg/Include/Uefi/UefiBaseType.h to > >> MdePkg/Include/Base.h. > >> > >> So: > >> > >> typedef struct { > >> UINT32 Data1; > >> UINT16 Data2; > >> UINT16 Data3; > >> UINT8 Data4[8]; > >> } GUID; > >> > >> Becomes: > >> > >> typedef struct { > >> UINT32 Data1; > >> UINT16 Data2; > >> UINT16 Data3; > >> UINT8 Data4[8]; > >> } GUID, EFI_GUID; > >> > >> Thanks, > >> > >> Andrew Fish > >> > >> > >>> -----Original Message----- > >>> From: Kinney, Michael D > >> [mailto:michael.d.kinney@intel.com] > >>> Sent: Thursday, January 17, 2019 3:04 PM > >>> To: Felix Polyudov; 'Andrew Fish'; Kinney, Michael D > >>> Cc: edk2-devel@lists.01.org > >>> Subject: RE: [edk2] History question about Base.h > and > >> its alternate parallel name space.... Should we > change > >> it? > >>> > >>> Felix, > >>> > >>> Is there a specific set that would have the most > >> benefit? > >>> > >>> Is EFI_GUID sufficient? > >>> > >>> Mike > >>> > >>>> -----Original Message----- > >>>> From: Felix Polyudov [mailto:Felixp@ami.com] > >>>> Sent: Wednesday, January 16, 2019 3:10 PM > >>>> To: 'Andrew Fish' ; Kinney, > Michael > >> D > >>>> > >>>> Cc: edk2-devel@lists.01.org > >>>> Subject: RE: [edk2] History question about Base.h > and > >>>> its alternate parallel name space.... Should we > >> change > >>>> it? > >>>> > >>>> I agree with Andrew. > >>>> In my opinion, moving alias types to Base.h will > help > >> to > >>>> overcome certain practical inconveniences without > >>>> a significant impact on the ability to detect > poorly > >>>> written Base libraries. > >>>> > >>>> -----Original Message----- > >>>> From: edk2-devel [mailto:edk2-devel- > >>>> bounces@lists.01.org] On Behalf Of Andrew Fish via > >> edk2- > >>>> devel > >>>> Sent: Wednesday, January 16, 2019 5:18 PM > >>>> To: Mike Kinney > >>>> Cc: edk2-devel@lists.01.org > >>>> Subject: Re: [edk2] History question about Base.h > and > >>>> its alternate parallel name space.... Should we > >> change > >>>> it? > >>>> > >>>> > >>>> > >>>>> On Jan 16, 2019, at 1:19 PM, Kinney, Michael D > >>>> wrote: > >>>>> > >>>>> Hi Andrew, > >>>>> > >>>>> I though the reason was a bit more technical. We > >> have > >>>> a > >>>>> MODULE_TYPE of BASE. Library instances that use > the > >>>> BASE > >>>>> MODULE_TYPE are declaring that the library > >> interfaces > >>>> are > >>>>> safe to be linked against a module of any other > type > >>>> (SEC, > >>>>> PEI, DXE, SMM, DXE_RUNTIME, UEFI_DRIVER, > UEFI_APP). > >>>>> > >>>>> We needed to make sure that a lib of type BASE > that > >>>>> includes Base.h as its top level include file only > >> has > >>>>> visibility to the types that are safe for all the > >>>> other > >>>>> module types. It is up to the top level include > >> files > >>>>> for these other module types to provide the gasket > >> to > >>>>> the types in Base.h. > >>>>> > >>>>> If we add aliases in Base.h, then we may not get > >> build > >>>>> breaks when a lib of type BASE includes files that > >> are > >>>>> not compatible with BASE. > >>>>> > >>>> > >>>> Mike, > >>>> > >>>> I don't think having aliases for return types > really > >>>> helps Base code quality as RETURN_SUCCESS is > almost > >>>> always just a comment in a header file, and only > >> exists > >>>> in a .c file. Thus RETURN_* seem like a needless > >>>> duplication, best case it is a comment that the > code > >> is > >>>> Base. > >>>> > >>>> I will agree that not having EFI_GUID defined does > >> case > >>>> all the PPI and Protocol files to blow up if you > try > >> to > >>>> include them. The failure case I was helping > explain > >> was > >>>> some one trying to include a PPI, that included a > >>>> Protocol that contained a data structure that was > >>>> needed. But I would posit that the definition of a > >>>> (EFI_)GUID is state agnostic. Having access to a > PPI > >> or > >>>> Protocol definition does not break Base code, what > >>>> breaks Base code is trying to access some service > >> that > >>>> does not exist. To get more that EFI_GUID you are > >> going > >>>> to need to include Uefi.h, PiPei.h, PiDxe.h, etc. > and > >>>> that will block doing anything that is not Base. > >>>> > >>>> So I'm asking if redefining the name for EFI_GUID > to > >>>> GUID for Base is really that helpful? > >>>> > >>>> Thanks, > >>>> > >>>> Andrew Fish > >>>> > >>>> > >>>>> Thanks, > >>>>> > >>>>> Mike > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: edk2-devel [mailto:edk2-devel- > >>>>>> bounces@lists.01.org] On Behalf Of Andrew Fish > via > >>>> edk2- > >>>>>> devel > >>>>>> Sent: Wednesday, January 16, 2019 1:00 PM > >>>>>> To: edk2-devel > >>>>>> Subject: [edk2] History question about Base.h and > >> its > >>>>>> alternate parallel name space.... Should we > change > >>>> it? > >>>>>> > >>>>>> I had some one ask me recently why EFI_GUID does > >> not > >>>>>> work with #include . I explained they > >> needed > >>>> to > >>>>>> use GUID vs. EFI_GUID. That prompted the question > >> of > >>>> why > >>>>>> we have 2 names for the same thing..... Well the > >>>>>> historical answer was kind of political as some > >> team > >>>>>> wanted to use edk2, but not implement EFI. Thus > we > >>>> have > >>>>>> EFI types without the EFI_ prefix in Base.h. > >>>>>> > >>>>>> So all this got me thinking.... Maybe it makes > >> sense > >>>> to > >>>>>> move some of the renaming from > >>>>>> MdePkg/Include/Uefi/UefiBaseType.h to Base.h? > >>>> Removing > >>>>>> the Base.h duplicate types would potentially hit > >> lots > >>>> of > >>>>>> code [1] and break merges with other code bases > >>>> (break > >>>>>> other peoples Base libs etc.). > >>>>>> > >>>>>> These lines in MdePkg/Include/Uefi/UefiBaseType.h > >>>> would > >>>>>> get moved to MdePkg/Include/Base.h: > >>>>>> typedef GUID EFI_GUID; > >>>>>> typedef RETURN_STATUS EFI_STATUS; > >>>>>> #define EFIERR(_a) > ENCODE_ERROR(_a) > >>>>>> #define EFI_ERROR(A) RETURN_ERROR(A) > >>>>>> > >>>>>> #define EFI_SUCCESS RETURN_SUCCESS > >>>>>> #define EFI_LOAD_ERROR > RETURN_LOAD_ERROR > >>>>>> #define EFI_INVALID_PARAMETER > >>>>>> RETURN_INVALID_PARAMETER > >>>>>> #define EFI_UNSUPPORTED > >> RETURN_UNSUPPORTED > >>>>>> #define EFI_BAD_BUFFER_SIZE > >>>> RETURN_BAD_BUFFER_SIZE > >>>>>> #define EFI_BUFFER_TOO_SMALL > >>>>>> RETURN_BUFFER_TOO_SMALL > >>>>>> #define EFI_NOT_READY > RETURN_NOT_READY > >>>>>> #define EFI_DEVICE_ERROR > >> RETURN_DEVICE_ERROR > >>>>>> #define EFI_WRITE_PROTECTED > >>>> RETURN_WRITE_PROTECTED > >>>>>> #define EFI_OUT_OF_RESOURCES > >>>>>> RETURN_OUT_OF_RESOURCES > >>>>>> #define EFI_VOLUME_CORRUPTED > >>>>>> RETURN_VOLUME_CORRUPTED > >>>>>> #define EFI_VOLUME_FULL > >> RETURN_VOLUME_FULL > >>>>>> #define EFI_NO_MEDIA RETURN_NO_MEDIA > >>>>>> #define EFI_MEDIA_CHANGED > >>>> RETURN_MEDIA_CHANGED > >>>>>> #define EFI_NOT_FOUND > RETURN_NOT_FOUND > >>>>>> #define EFI_ACCESS_DENIED > >>>> RETURN_ACCESS_DENIED > >>>>>> #define EFI_NO_RESPONSE > >> RETURN_NO_RESPONSE > >>>>>> #define EFI_NO_MAPPING > RETURN_NO_MAPPING > >>>>>> #define EFI_TIMEOUT RETURN_TIMEOUT > >>>>>> #define EFI_NOT_STARTED > >> RETURN_NOT_STARTED > >>>>>> #define EFI_ALREADY_STARTED > >>>> RETURN_ALREADY_STARTED > >>>>>> #define EFI_ABORTED RETURN_ABORTED > >>>>>> #define EFI_ICMP_ERROR > RETURN_ICMP_ERROR > >>>>>> #define EFI_TFTP_ERROR > RETURN_TFTP_ERROR > >>>>>> #define EFI_PROTOCOL_ERROR > >>>> RETURN_PROTOCOL_ERROR > >>>>>> #define EFI_INCOMPATIBLE_VERSION > >>>>>> RETURN_INCOMPATIBLE_VERSION > >>>>>> #define EFI_SECURITY_VIOLATION > >>>>>> RETURN_SECURITY_VIOLATION > >>>>>> #define EFI_CRC_ERROR > RETURN_CRC_ERROR > >>>>>> #define EFI_END_OF_MEDIA > >> RETURN_END_OF_MEDIA > >>>>>> #define EFI_END_OF_FILE > >> RETURN_END_OF_FILE > >>>>>> #define EFI_INVALID_LANGUAGE > >>>>>> RETURN_INVALID_LANGUAGE > >>>>>> #define EFI_COMPROMISED_DATA > >>>>>> RETURN_COMPROMISED_DATA > >>>>>> #define EFI_HTTP_ERROR > RETURN_HTTP_ERROR > >>>>>> > >>>>>> #define EFI_WARN_UNKNOWN_GLYPH > >>>>>> RETURN_WARN_UNKNOWN_GLYPH > >>>>>> #define EFI_WARN_DELETE_FAILURE > >>>>>> RETURN_WARN_DELETE_FAILURE > >>>>>> #define EFI_WARN_WRITE_FAILURE > >>>>>> RETURN_WARN_WRITE_FAILURE > >>>>>> #define EFI_WARN_BUFFER_TOO_SMALL > >>>>>> RETURN_WARN_BUFFER_TOO_SMALL > >>>>>> #define EFI_WARN_STALE_DATA > >>>> RETURN_WARN_STALE_DATA > >>>>>> #define EFI_WARN_FILE_SYSTEM > >>>>>> RETURN_WARN_FILE_SYSTEM > >>>>>> > >>>>>> I'm interested what folks think about a change > like > >>>>>> this? This change makes the alternate names > >> optional. > >>>>>> > >>>>>> I guess we could also leave the old Base.h > >>>> definitions > >>>>>> in Base.h and cleanup the code to only use the > EFI > >>>> form, > >>>>>> but that is a much bigger change? > >>>>>> > >>>>>> [1] RETURN_SUCCSS usage: git grep -w > RETURN_SUCCESS > >>>>>> > >>>>>> Thanks, > >>>>>> > >>>>>> Andrew Fish > >>>>>> > >>>>>> _______________________________________________ > >>>>>> edk2-devel mailing list > >>>>>> edk2-devel@lists.01.org > >>>>>> https://lists.01.org/mailman/listinfo/edk2-devel > >>>> > >>>> _______________________________________________ > >>>> edk2-devel mailing list > >>>> edk2-devel@lists.01.org > >>>> https://lists.01.org/mailman/listinfo/edk2-devel > >>>> > >>>> Please consider the environment before printing > this > >>>> email. > >>>> > >>>> The information contained in this message may be > >>>> confidential and proprietary to American > Megatrends, > >>>> Inc. This communication is intended to be read > only > >> by > >>>> the individual or entity to whom it is addressed or > >> by > >>>> their designee. If the reader of this message is > not > >> the > >>>> intended recipient, you are on notice that any > >>>> distribution of this message, in any form, is > >> strictly > >>>> prohibited. Please promptly notify the sender by > >> reply > >>>> e-mail or by telephone at 770-246-8600, and then > >> delete > >>>> or destroy all copies of the transmission. > >>> > >>> Please consider the environment before printing this > >> email. > >>> > >>> The information contained in this message may be > >> confidential and proprietary to American Megatrends, > >> Inc. This communication is intended to be read only > by > >> the individual or entity to whom it is addressed or > by > >> their designee. If the reader of this message is not > the > >> intended recipient, you are on notice that any > >> distribution of this message, in any form, is > strictly > >> prohibited. Please promptly notify the sender by > reply > >> e-mail or by telephone at 770-246-8600, and then > delete > >> or destroy all copies of the transmission. > >