Re: [libvirt] [RFC PATCH 5/6] qemu: Switch over command line capabilities to virBitmap
On 02/10/2011 06:48 AM, Jiri Denemark wrote:
> On 02/09/2011 09:02 AM, Jiri Denemark wrote:
>> This is done for two reasons:
>> - we are getting very close to 64 flags which is the maximum we can use
>> with unsigned long long
>> - by using LL constants in enum we already violates C99 constraint that
>> enum values have to fit into int
>
> Does gcc warn about that (possible via some -W flag that we aren't
> currently using)?
It doesn't warn about it by default and I'm not aware of any flag
which
would
turn this warning on.
-pedantic
But that's a heavy hammer. So I filed a gcc bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47701
This is mostly for two reasons. First, gcc only warns on
ATTRIBUTE_NONNULL if
NULL is explicitly passed as a parameter and not if it's done through a
variable. So it doesn't really help with detecting cases where NULL is
indirectly passed to qemuCapsGet().
That's likewise a bug in gcc:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308
Meanwhile, clang DOES do better NULL analysis, and will warn where gcc
currently doesn't. It's just that we don't build under clang as often.
So just because gcc doesn't exploit the attribute as well as it should
is not a reason to avoid use of the attribute.
>> -bool qemuCapsGet(unsigned long long caps,
>> +bool qemuCapsGet(virBitmapPtr caps,
>> enum qemuCapsFlags flag);
>
> I guess you really did intend for qemuCapsGet to work even if
> qemuCapsFree failed.
I didn't get this last comment. Could you be more specific about what you
wanted to say? :-)
I was noticing that you omitted the ATTRIBUTE_NONNULL check in the
header and having the explicit NULL check in the code, and therefore
assuming that you meant to do it this way (if you had marked this
ATTRIBUTE_NONNULL, then the check in the code would have been
redundant). But you've convinced me that we have at least one case
where we DO pass a null bitset, and so this part of the patch is
correct; nothing to change.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)