[libvirt] [PATCH] tests: do not ignore virInitialize failure

Eric Blake eblake at redhat.com
Tue May 18 15:39:02 UTC 2010


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 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: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100518/912a116a/attachment-0001.sig>


More information about the libvir-list mailing list