On 02/12/14 00:42, Laszlo Ersek wrote: > comments below > > On 02/11/14 18:16, Jordan Justen wrote: >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jordan Justen <[email protected]> >> --- >> OvmfPkg/Library/LoadLinuxLib/Linux.c | 4 ++-- >> OvmfPkg/PlatformPei/MemDetect.c | 2 +- >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c | 3 ++- >> OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 6 ++++-- >> OvmfPkg/VirtioNetDxe/DriverBinding.c | 2 +- >> OvmfPkg/VirtioNetDxe/SnpGetStatus.c | 2 +- >> OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 2 +- >> 7 files changed, 12 insertions(+), 9 deletions(-) >> >> diff --git a/OvmfPkg/Library/LoadLinuxLib/Linux.c >> b/OvmfPkg/Library/LoadLinuxLib/Linux.c >> index 37b14f5..89e5cd4 100644 >> --- a/OvmfPkg/Library/LoadLinuxLib/Linux.c >> +++ b/OvmfPkg/Library/LoadLinuxLib/Linux.c >> @@ -384,8 +384,8 @@ SetupLinuxMemmap ( >> #ifdef MDE_CPU_IA32 >> Efi->efi_loader_signature = SIGNATURE_32 ('E', 'L', '3', '2'); >> #else >> - Efi->efi_systab_hi = ((UINT64)(UINTN) gST) >> 32; >> - Efi->efi_memmap_hi = ((UINT64)(UINTN) MemoryMapPtr) >> 32; >> + Efi->efi_systab_hi = (UINT32) (((UINT64)(UINTN) gST) >> 32); >> + Efi->efi_memmap_hi = (UINT32) (((UINT64)(UINTN) MemoryMapPtr) >> 32); >> Efi->efi_loader_signature = SIGNATURE_32 ('E', 'L', '6', '4'); >> #endif >> >> diff --git a/OvmfPkg/PlatformPei/MemDetect.c >> b/OvmfPkg/PlatformPei/MemDetect.c >> index 29df537..fe05fa4 100644 >> --- a/OvmfPkg/PlatformPei/MemDetect.c >> +++ b/OvmfPkg/PlatformPei/MemDetect.c >> @@ -55,7 +55,7 @@ GetSystemMemorySizeBelow4gb ( >> Cmos0x34 = (UINT8) CmosRead8 (0x34); >> Cmos0x35 = (UINT8) CmosRead8 (0x35); >> >> - return (((UINTN)((Cmos0x35 << 8) + Cmos0x34) << 16) + SIZE_16MB); >> + return (UINT32) (((UINTN)((Cmos0x35 << 8) + Cmos0x34) << 16) + SIZE_16MB); >> } >> >> >> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c >> b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c >> index 7d26c41..62065d1 100644 >> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c >> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FwBlockService.c >> @@ -879,7 +879,8 @@ Returns: >> if (Checksum != 0) { >> UINT16 Expected; >> >> - Expected = ((UINTN) FwVolHeader->Checksum + 0x10000 - Checksum) & >> 0xffff; >> + Expected = (UINT16) ((UINTN) FwVolHeader->Checksum + 0x10000 - >> Checksum) & >> + 0xffff; > > This is "almost" an incorrect modification, because the new cast binds > with higher precedence than the bitwise and. In practice the error > doesn't matter, but I'd suggest to drop the masking with 0xffff.
This is the only point that I'd like to keep from my first review of this patch. Ie. please drop the "& 0xffff" part, and then add my Reviewed-by: Laszlo Ersek <[email protected]> Thanks Laszlo ------------------------------------------------------------------------------ Android apps run on BlackBerry 10 Introducing the new BlackBerry 10.2.1 Runtime for Android apps. Now with support for Jelly Bean, Bluetooth, Mapview and more. Get your Android app in front of a whole new audience. Start now. http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
