On Tue, May 18, 2010 at 05:25:20PM +0200, Jim Meyering wrote:
Eric Blake wrote:
> On 05/18/2010 04:33 AM, Jim Meyering wrote:
>> Simple...
>>
>>>From f5ee09ed08473478b3ea3135d51125fbf687e402 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering(a)redhat.com>
>> Date: Tue, 18 May 2010 12:32:39 +0200
>> Subject: [PATCH] tests: do not ignore virInitialize failure
>>
>> * tests/nodeinfotest.c (mymain): Do not ignore virInitialize failure.
>> Most other callers of virInitialize test for failure.
>> ---
>> tests/nodeinfotest.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/tests/nodeinfotest.c b/tests/nodeinfotest.c
>> index ff056b9..d3c500d 100644
>> --- a/tests/nodeinfotest.c
>> +++ b/tests/nodeinfotest.c
>> @@ -106,7 +106,8 @@ mymain(int argc, char **argv)
>> return(EXIT_FAILURE);
>> }
>>
>> - virInitialize();
>> + if (virInitialize() < 0)
>> + return EXIT_FAILURE;
>
> 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 ?
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://deltacloud.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|