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

Jim Meyering jim at meyering.net
Tue May 18 15:46:14 UTC 2010


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.




More information about the libvir-list mailing list