[libvirt] [PATCH 1/3] Use virGetLastErrorMessage to avoid Coverity message
John Ferlan
jferlan at redhat.com
Sat May 7 16:35:11 UTC 2016
On 05/07/2016 11:04 AM, Erik Skultety wrote:
> On 06/05/16 21:34, John Ferlan wrote:
>> Both instances use VIR_WARN() to print the error from a failed
>> virDBusGetSystemBus() call. Rather than use the virGetLastError
>> and need to check for valid return err pointer, just use the
>> virGetLastErrorMessage.
>>
>
> I'm afraid these are not equivalent. virGetLastError states that it
> returns NULL if no error occurred, which isn't true in all the cases,
> i.e. both virGetLastError and virGetLastErrorMessage call
> virLastErrorObject which can actually fail when no threadlocal error
> object was found (this is OK), but then we try to create an empty one
> and assign it to the threadlocal storage, so if the allocation fails
> quietly (I think trying to log a proper message would be least of our
> concerns), but if setting the new, empty error object as thread-specific
> data fails, it will return NULL which virGetLastError just passes along
> and the original caller deals with this by checking the pointer and if
> it's NULL, "Unknown error" is used instead. Now, in your case however,
> should such a corner-case scenario happen, virGetLastErrorMessage
> interprets the NULL from virLastErrorObject as "no error" which isn't
> quite the same, it might even get a little confusing back at the client
> side. So I prefer we stick to the "checking" convention, unless we want
> to make some adjustments to the virerror module.
>
> Erik
>
Just to followup on Cole's post... The bite size tasks is where the
"idea" comes from:
http://wiki.libvirt.org/page/BiteSizedTasks#More_usage_of_virGetLastErrorMessage.28.29
John
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/network/bridge_driver.c | 3 +--
>> src/node_device/node_device_hal.c | 3 +--
>> 2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index a34da2a..f406f04 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -695,9 +695,8 @@ networkStateInitialize(bool privileged,
>>
>> #ifdef HAVE_FIREWALLD
>> if (!(sysbus = virDBusGetSystemBus())) {
>> - virErrorPtr err = virGetLastError();
>> VIR_WARN("DBus not available, disabling firewalld support "
>> - "in bridge_network_driver: %s", err->message);
>> + "in bridge_network_driver: %s", virGetLastErrorMessage());
>> } else {
>> /* add matches for
>> * NameOwnerChanged on org.freedesktop.DBus for firewalld start/stop
>> diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
>> index 6d18a87..6ddfad0 100644
>> --- a/src/node_device/node_device_hal.c
>> +++ b/src/node_device/node_device_hal.c
>> @@ -641,9 +641,8 @@ nodeStateInitialize(bool privileged ATTRIBUTE_UNUSED,
>>
>> dbus_error_init(&err);
>> if (!(sysbus = virDBusGetSystemBus())) {
>> - virErrorPtr verr = virGetLastError();
>> VIR_ERROR(_("DBus not available, disabling HAL driver: %s"),
>> - verr->message);
>> + virGetLastErrorMessage());
>> ret = 0;
>> goto failure;
>> }
>>
>
More information about the libvir-list
mailing list