From: "Kinney, Michael D" <michael.d.kinney@intel.com>
To: Felix Polyudov <Felixp@ami.com>, 'Andrew Fish' <afish@apple.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: History question about Base.h and its alternate parallel name space.... Should we change it?
Date: Thu, 17 Jan 2019 20:04:02 +0000 [thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B8B846FA@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <9333E191E0D52B4999CE63A99BA663A00302C56A66@atlms1.us.megatrends.com>
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' <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> 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
> <michael.d.kinney@intel.com> 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 <edk2-devel@lists.01.org>
> >> 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 <Base.h>. 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.
next prev parent reply other threads:[~2019-01-17 20:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-16 21:00 History question about Base.h and its alternate parallel name space.... Should we change it? Andrew Fish
2019-01-16 21:19 ` Kinney, Michael D
2019-01-16 22:18 ` Andrew Fish
2019-01-16 23:09 ` Felix Polyudov
2019-01-17 20:04 ` Kinney, Michael D [this message]
2019-01-18 17:08 ` Felix Polyudov
2019-01-18 17:24 ` Andrew Fish
2019-01-22 17:49 ` Kinney, Michael D
2019-01-23 1:00 ` Andrew Fish
2019-01-23 18:27 ` Kinney, Michael D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E92EE9817A31E24EB0585FDF735412F5B8B846FA@ORSMSX113.amr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox