Re: [libvirt] [PATCH] tests: do not ignore virInitialize failure
On 05/18/2010 09:35 AM, Daniel P. Berrange wrote:
>> Shouldn't we be adding ATTRIBUTE_RETURN_CHECK to
virInitialize in the
>> appropriate header, to let the compiler enforce us to do the checking?
>
> That would be nice, but the declaration of virInitialize
> is in libvirt.h.in, and I am leery of adding new symbols in such
> an exposed header, in spite of the few that are already there, e.g.,
Arguably every single function in the public libvirt.h should
have an ATTRIBUTE_RETURN_CHECK annotation. The downside is that
we could cause compile errors for existing apps using libvirt if
they have been sloppy. I'm in two minds as to whether that's
acceptable or not. Perhaps we could turn it on only if the symbol
FORTIFY_SOURCE was defined, or something similar ?
ATTRIBUTE_RETURN_CHECK only causes a warning, unless you are compiling
with -Werror. If users were sloppy, either they fix their coding, or
they disable -Werror.
I see no problem with adding ATTRIBUTE_RETURN_CHECK globally (witness
how recent glibc has been adding it to a lot of standard interfaces,
without regards to FORTIFY_SOURCE). I see it as a service to our users.
But I also agree with Jim's sentiment that adding it is a separate
patch; therefore, ACK to the tests/nodeinfotest.c change regardless of
whether we modify libvirt.h.in in a separate patch.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)