[libvirt] [RFC PATCH 5/6] qemu: Switch over command line capabilities to virBitmap

[Eric Blake]

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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110211/350a1695/attachment-0001.sig>