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@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