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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list