[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]