Laszlo Ersek [mailto:[email protected]] wrote:
[...]
]> 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.
This would be true if EDK2 defined BOOLEAN as a real C99 _Bool. But for
compatibility with VS2005, EDK2 typedefs BOOLEAN to uint8_t. As a
result, the following always evaluates to zero:
(BOOLEAN) (Features & VIRTIO_BLK_F_FLUSH);
This is because VIRTIO_BLK_F_FLUSH is 0x200 and casting the bitwise and
result to BOOLEAN is the same as casting 0 or 0x200 to uint8_t. On
the other hand, this C99 code gives the intended result:
(_Bool) (Features & VIRTIO_BLK_F_FLUSH);
Anyone who is accustomed to the behavior of C99 _Bool (bool) must remember
that EDK2 BOOLEAN is something different (a cast to uint8_t).
]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.
Yes, best case working readable code becomes working ugly code. Worse
case, as shown above, is working readable code becomes code that is not
only ugly, but broken too. Adding type casts to silence a Microsoft
warning can break working code in a way that may not be immediately
obvious. Because the cast is intended to silence a warning rather than
alter code generation, it makes more sense to disable that warning
through the command line than to modify the working code.
]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?
Certainly true. This or a similar change could be made to
BaseTools/Conf/tools_def.template so that the problem is eliminated
not only for OVMF, but for all other code. A tools_def.template
could target VS2005 only.
Thanks,
Scott
](NB this is not just knee-jerk MSVC hate on my part. I loathe the
]default -Werror for gcc in qemu just the same.)
[...]
]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
