On Feb 8, 2013, at 10:49 AM, David Woodhouse <[email protected]> wrote:

> On Fri, 2013-02-08 at 10:18 -0800, Jordan Justen wrote:
>> 
>> I'm thinking I'd prefer:
>> (Bp->hdr.xloadflags & ((sizeof(UINTN) == 4) ? BIT2 : BIT3))
>> 
>> This matches the kernel comment.
>> 
>> That okay David?
> 
> As long as it works. Although if you're going that far you might as well
> go the whole hog and do something completely long-hand and ugly like
> 
> #ifdef MDE_CPU_IA32
> #define EFI_SUPPORTED_BIT BIT2
> #else
> #define EFI_SUPPORTED_BIT BIT3
> #endif
> 

This breaks for ARM, and potentially any new 32-bit CPU that gets added to EFI, 
so I don't like this construct.  It should test for processor types explicitly 
and have an #else #error. 

>From an EFI ABI point of view sizeof(UINTN) == 4 is 32-bit CPU and 
>sizeof(UINTN) == 8 is 64-bit CPU. So maybe it would be better to make the 
>((sizeof(UINTN) == 4) ? BIT2 : BIT3) a #define with a good comment. 

It is also possible to remove the #ifdef and have IA32/CpuFlags.h and 
X64/CpuFlags.h file that just set the #define. If the module does #include 
<CpuFlag.h> it will get the correct version since the build system adds the 
current processor architecture type as an include directory path. This will 
also break if you port to a new processor type, and it is obvious when you are 
porting the module it needs to change. 

Thanks,

Andrew Fish



------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013 
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to