Yes, I understood. I just think that all values must be UINT32 because I can't
imaging that EndLBA will be > Uint32Max. It will be bug for any disk
software/drivers.
On 27.11.2014, at 11:55, Tian, Feng wrote:
> Sergey, You misunderstand what I meant.
>
> I am talking about EndingLBA rather than StartingLBA and SizeInLBA. EndingLBA
> = StartingLBA + SizeInLBA. So EndingLBA could be > 0xFFFFFFFF and up to 2T
> space.
>
> From: Sergey Isakov [mailto:[email protected]]
> Sent: Thursday, November 27, 2014 16:16
> To: [email protected]
> Subject: Re: [edk2] [PATCH] MdeModulePkg : Misc comments and DEBUG messages
>
> Feng,
> IndustryStandard assumes UINT32
> ----
> typedef struct {
> UINT8 BootIndicator;
> UINT8 StartHead;
> UINT8 StartSector;
> UINT8 StartTrack;
> UINT8 OSIndicator;
> UINT8 EndHead;
> UINT8 EndSector;
> UINT8 EndTrack;
> UINT8 StartingLBA[4];
> UINT8 SizeInLBA[4];
> } MBR_PARTITION_RECORD;
>
> ----
> Sergey
>
> On 27.11.2014, at 9:00, Tian, Feng wrote:
>
>
> 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]
> 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]
> 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]
> 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]>
>
> Thanks
> Feng
>
> From: El-Haj-Mahmoud, Samer [mailto:[email protected]]
> Sent: Wednesday, November 26, 2014 06:55
> To: [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]
>
>
> Thanks,
>
> Samer El-Haj-Mahmoud
> System Firmware Architect
> HP Servers
>
> [email protected]
> T +1.281.514.5973
> C +1.512.659.1523
> Hewlett-Packard Company
> hp.com/go/proliant/uefi
>
> <image001.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
>
> ------------------------------------------------------------------------------
> 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
------------------------------------------------------------------------------
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
