public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
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.


  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