Re: History question about Base.h and its alternate parallel name space.... Should we change it?

[Andrew Fish <afish@apple.com>]

> 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