Scott,
Thanks for reminder. The number one (1) and letter l (l) is so hard to
distinguish that my subconscious treats this format string in debug message as
%lx (letter L)…
Samer,
Could you help clarify what’s your original intention for this debug message?
Do you want to use “l” or “1”?
And I found there is another bug in original code. EndingLBA local variable was
defined as UINT32. But it should be UINT64 as the sum of two UINT32 integers
may beyond the scope of UINT32. (see below code)
UINT32 StartingLBA;
UINT32 EndingLBA; <- should be UINT64
EndingLBA = StartingLBA + UNPACK_UINT32 (Mbr->Partition[Index1].SizeInLBA) -
1; <- UINT32 + UINT32 may larger than the scope of UINT32. So EndingLBA should
be UINT64 type.
Thanks
Feng
From: Scott Duplichan [mailto:[email protected]]
Sent: Thursday, November 27, 2014 10:24
To: [email protected]
Subject: Re: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
I just noticed the new statement:
DEBUG((EFI_D_INFO, "PartitionValidMbr: Bad MBR partition size
EndingLBA(%1x) > LastLBA(%1x)\n", EndingLBA, LastLba));
Both arguments are printed with the same %1x format, though the first is UINT32
while the second is EFI_LBA (UINT64). I believe that is wrong, though it might
end up printing correctly as long as EDK2 is never built for big endian.
The use of %1x itself is curious. I believe %1x is equivalent to %x. For that
reason, %1x shows up nowhere else in the EDK2 tree. On the other hand, %lx
appears in 88 different C files. It almost looks like the intent was
EndingLBA(%lx) > LastLBA(%lx), though that is incorrect, too.
Thanks,
Scott
From: Tian, Feng [mailto:[email protected]]
Sent: Wednesday, November 26, 2014 07:03 PM
To: [email protected]<mailto:[email protected]>
Subject: Re: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
Checked in at r16450.
From: El-Haj-Mahmoud, Samer [mailto:[email protected]]
Sent: Wednesday, November 26, 2014 23:50
To: [email protected]<mailto:[email protected]>
Subject: Re: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
Thanks Feng. Attached is the updated patch with the suggested changes.
Please confirm once this is committed.
Thanks,
--Samer
From: Tian, Feng [mailto:[email protected]]
Sent: Tuesday, November 25, 2014 6:44 PM
To: [email protected]<mailto:[email protected]>
Subject: Re: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
Minor comment:
The first debug message misses a line break sign and I think you can directly
use string “I2cHostI2cBusConfigurationAvailable” in format string rather than
%a.
Others look good to me.
Reviewed-by: Feng Tian <[email protected]<mailto:[email protected]>>
Thanks
Feng
From: El-Haj-Mahmoud, Samer [mailto:[email protected]]
Sent: Wednesday, November 26, 2014 06:55
To: [email protected]<mailto:[email protected]>
Subject: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
Dear MdeModulePkg maintainers,
Please see attached patch
Fixed some spelling typos in some comments. Added a couple of useful DEBUG
messages
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud [email protected]<mailto:[email protected]>
Thanks,
Samer El-Haj-Mahmoud
System Firmware Architect
HP Servers
[email protected]<mailto:[email protected]>
T +1.281.514.5973
C +1.512.659.1523
Hewlett-Packard Company
hp.com/go/proliant/uefi<http://hp.com/go/proliant/uefi>
[Description: Description: C:\Users\elhajmah\HpLogo.png]
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
