Eric Blake wrote: > 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. If we go forward with this, let's avoid using that particular name and instead use something more namespace-friendly like glibc's __wur or __attribute_warn_unused_result__: /usr/include/sys/cdefs.h:# define __wur __attribute_warn_unused_result__ /usr/include/sys/cdefs.h:# define __wur /* Ignore */ > 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. I agree. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list