r-b with your suggestions adopted? On Tue, Feb 11, 2014 at 3:42 PM, Laszlo Ersek <[email protected]> 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. > >> >> DEBUG ((EFI_D_INFO, "FV@%p Checksum is 0x%x, expected 0x%x\n", >> FwVolHeader, FwVolHeader->Checksum, Expected)); >> diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c >> b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c >> index 35fc88e..2561c52 100644 >> --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c >> +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c >> @@ -770,8 +770,10 @@ VirtioBlkInit ( >> Dev->BlockIoMedia.RemovableMedia = FALSE; >> Dev->BlockIoMedia.MediaPresent = TRUE; >> Dev->BlockIoMedia.LogicalPartition = FALSE; >> - Dev->BlockIoMedia.ReadOnly = !!(Features & VIRTIO_BLK_F_RO); >> - Dev->BlockIoMedia.WriteCaching = !!(Features & VIRTIO_BLK_F_FLUSH); >> + Dev->BlockIoMedia.ReadOnly = (BOOLEAN) !!(Features & >> + VIRTIO_BLK_F_RO); >> + Dev->BlockIoMedia.WriteCaching = (BOOLEAN) !!(Features & >> + VIRTIO_BLK_F_FLUSH); > > No need for the double logical negation (ie. !!) when there is an > explicit cast to BOOLEAN. > > BTW VS2005 should *really* know that the logical negation operator can > only return 0 or 1. Honestly, trying to comply with such an old and > relatively primitive code analyzer actively hurts our code, and wastes > our (human) resources. You fix it up, I review it, the code gets uglier. > Pure loss for everyone. > > If I remember correctly, the INF files allow local overriding of > compiler flags. I think we should look into disabling "warnings are > errors". Is there some MSVC flag that we can just tack on to the > existing command line in the INF files and it reduces warnings to > warnings again? > > (NB this is not just knee-jerk MSVC hate on my part. I loathe the > default -Werror for gcc in qemu just the same.) > >> Dev->BlockIoMedia.BlockSize = BlockSize; >> Dev->BlockIoMedia.IoAlign = 0; >> Dev->BlockIoMedia.LastBlock = DivU64x32 (NumSectors, >> diff --git a/OvmfPkg/VirtioNetDxe/DriverBinding.c >> b/OvmfPkg/VirtioNetDxe/DriverBinding.c >> index 93995c6..13f73c1 100644 >> --- a/OvmfPkg/VirtioNetDxe/DriverBinding.c >> +++ b/OvmfPkg/VirtioNetDxe/DriverBinding.c >> @@ -129,7 +129,7 @@ VirtioNetGetFeatures ( >> if (EFI_ERROR (Status)) { >> goto YieldDevice; >> } >> - *MediaPresent = !!(LinkStatus & VIRTIO_NET_S_LINK_UP); >> + *MediaPresent = (BOOLEAN) !!(LinkStatus & VIRTIO_NET_S_LINK_UP); >> } > > Again, the cast obviates "!!". > >> >> YieldDevice: >> diff --git a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c >> b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c >> index 4393d24..fcf43f5 100644 >> --- a/OvmfPkg/VirtioNetDxe/SnpGetStatus.c >> +++ b/OvmfPkg/VirtioNetDxe/SnpGetStatus.c >> @@ -94,7 +94,7 @@ VirtioNetGetStatus ( >> if (EFI_ERROR (Status)) { >> goto Exit; >> } >> - Dev->Snm.MediaPresent = !!(LinkStatus & VIRTIO_NET_S_LINK_UP); >> + Dev->Snm.MediaPresent = (BOOLEAN) !!(LinkStatus & VIRTIO_NET_S_LINK_UP); >> } >> >> // > > same > >> diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c >> b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c >> index e6154cd..c6f3c8e 100644 >> --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c >> +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c >> @@ -748,7 +748,7 @@ VirtioScsiInit ( >> if (EFI_ERROR (Status)) { >> goto Failed; >> } >> - Dev->InOutSupported = !!(Features & VIRTIO_SCSI_F_INOUT); >> + Dev->InOutSupported = (BOOLEAN) !!(Features & VIRTIO_SCSI_F_INOUT); >> >> Status = VIRTIO_CFG_READ (Dev, MaxChannel, &MaxChannel); >> if (EFI_ERROR (Status)) { >> > > ditto > > 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
------------------------------------------------------------------------------ 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
