[libvirt] [PATCH 1/1] Fix issue found by coverity and cleanup

Coverity found an issue in lxc_driver and uml_driver that we don't check the return value of register functions. I've also updated all other places and unify the way we check the return value. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/bhyve/bhyve_driver.c | 6 ++++-- src/interface/interface_backend_netcf.c | 3 ++- src/lxc/lxc_driver.c | 6 ++++-- src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 9 ++++++--- src/qemu/qemu_driver.c | 6 ++++-- src/remote/remote_driver.c | 24 ++++++++++++++++-------- src/secret/secret_driver.c | 6 ++++-- src/storage/storage_driver.c | 3 ++- src/uml/uml_driver.c | 6 ++++-- 10 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9dbb299..f1ed510 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -715,7 +715,9 @@ static virStateDriver bhyveStateDriver = { int bhyveRegister(void) { - virRegisterDriver(&bhyveDriver); - virRegisterStateDriver(&bhyveStateDriver); + if (virRegisterDriver(&bhyveDriver) < 0) + return -1; + if (virRegisterStateDriver(&bhyveStateDriver) < 0) + return -1; return 0; } diff --git a/src/interface/interface_backend_netcf.c b/src/interface/interface_backend_netcf.c index b4c1fe9..98ce83b 100644 --- a/src/interface/interface_backend_netcf.c +++ b/src/interface/interface_backend_netcf.c @@ -1194,6 +1194,7 @@ int netcfIfaceRegister(void) { _("failed to register netcf interface driver")); return -1; } - virRegisterStateDriver(&interfaceStateDriver); + if (virRegisterStateDriver(&interfaceStateDriver) < 0) + return -1; return 0; } diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 0ab1ba2..27c27d8 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -5767,7 +5767,9 @@ static virStateDriver lxcStateDriver = { int lxcRegister(void) { - virRegisterDriver(&lxcDriver); - virRegisterStateDriver(&lxcStateDriver); + if (virRegisterDriver(&lxcDriver) < 0) + return -1; + if (virRegisterStateDriver(&lxcStateDriver) < 0) + return -1; return 0; } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c797f8f..181541e 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3232,7 +3232,8 @@ static virStateDriver networkStateDriver = { int networkRegister(void) { if (virRegisterNetworkDriver(&networkDriver) < 0) return -1; - virRegisterStateDriver(&networkStateDriver); + if (virRegisterStateDriver(&networkStateDriver) < 0) + return -1; return 0; } diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 2e89d07..228794d 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -716,8 +716,11 @@ static virDomainConfNWFilterDriver domainNWFilterDriver = { int nwfilterRegister(void) { - virRegisterNWFilterDriver(&nwfilterDriver); - virRegisterStateDriver(&stateDriver); - virDomainConfNWFilterRegister(&domainNWFilterDriver); + if (virRegisterNWFilterDriver(&nwfilterDriver) < 0) + return -1; + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; + if (virDomainConfNWFilterRegister(&domainNWFilterDriver) < 0) + return -1; return 0; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 89f443f..fc382a5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16796,7 +16796,9 @@ static virStateDriver qemuStateDriver = { }; int qemuRegister(void) { - virRegisterDriver(&qemuDriver); - virRegisterStateDriver(&qemuStateDriver); + if (virRegisterDriver(&qemuDriver) < 0) + return -1; + if (virRegisterStateDriver(&qemuStateDriver) < 0) + return -1; return 0; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..c9711bb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7825,15 +7825,23 @@ remoteRegister(void) { remoteDriver = &remote_driver; - if (virRegisterDriver(&remote_driver) == -1) return -1; - if (virRegisterNetworkDriver(&network_driver) == -1) return -1; - if (virRegisterInterfaceDriver(&interface_driver) == -1) return -1; - if (virRegisterStorageDriver(&storage_driver) == -1) return -1; - if (virRegisterNodeDeviceDriver(&node_device_driver) == -1) return -1; - if (virRegisterSecretDriver(&secret_driver) == -1) return -1; - if (virRegisterNWFilterDriver(&nwfilter_driver) == -1) return -1; + if (virRegisterDriver(&remote_driver) < 0) + return -1; + if (virRegisterNetworkDriver(&network_driver) < 0) + return -1; + if (virRegisterInterfaceDriver(&interface_driver) < 0) + return -1; + if (virRegisterStorageDriver(&storage_driver) < 0) + return -1; + if (virRegisterNodeDeviceDriver(&node_device_driver) < 0) + return -1; + if (virRegisterSecretDriver(&secret_driver) < 0) + return -1; + if (virRegisterNWFilterDriver(&nwfilter_driver) < 0) + return -1; #ifdef WITH_LIBVIRTD - if (virRegisterStateDriver(&state_driver) == -1) return -1; + if (virRegisterStateDriver(&state_driver) == -1) < 0) + return -1; #endif return 0; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 9f7f946..5cb6391 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -1194,7 +1194,9 @@ static virStateDriver stateDriver = { int secretRegister(void) { - virRegisterSecretDriver(&secretDriver); - virRegisterStateDriver(&stateDriver); + if (virRegisterSecretDriver(&secretDriver) < 0) + return -1; + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; return 0; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7dbff6c..466ceba 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2696,7 +2696,8 @@ int storageRegister(void) { if (virRegisterStorageDriver(&storageDriver) < 0) return -1; - virRegisterStateDriver(&stateDriver); + if (virRegisterStateDriver(&stateDriver) < 0) + return -1; return 0; } diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 3496e52..8ddf181 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2909,7 +2909,9 @@ static virStateDriver umlStateDriver = { }; int umlRegister(void) { - virRegisterDriver(¨Driver); - virRegisterStateDriver(¨StateDriver); + if (virRegisterDriver(¨Driver) < 0) + return -1; + if (virRegisterStateDriver(¨StateDriver) < 0) + return -1; return 0; } -- 1.8.3.1

On Mon, Mar 17, 2014 at 03:22:11PM +0100, Pavel Hrdina wrote:
Coverity found an issue in lxc_driver and uml_driver that we don't check the return value of register functions.
I've also updated all other places and unify the way we check the return value.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/bhyve/bhyve_driver.c | 6 ++++-- src/interface/interface_backend_netcf.c | 3 ++- src/lxc/lxc_driver.c | 6 ++++-- src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 9 ++++++--- src/qemu/qemu_driver.c | 6 ++++-- src/remote/remote_driver.c | 24 ++++++++++++++++-------- src/secret/secret_driver.c | 6 ++++-- src/storage/storage_driver.c | 3 ++- src/uml/uml_driver.c | 6 ++++-- 10 files changed, 48 insertions(+), 24 deletions(-)
ACK How about adding ATTRIBUTE_RETURN_CHECK onto all the virRegister function declarations too, so gcc enforces this. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 17.3.2014 15:36, Daniel P. Berrange wrote:
On Mon, Mar 17, 2014 at 03:22:11PM +0100, Pavel Hrdina wrote:
Coverity found an issue in lxc_driver and uml_driver that we don't check the return value of register functions.
I've also updated all other places and unify the way we check the return value.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/bhyve/bhyve_driver.c | 6 ++++-- src/interface/interface_backend_netcf.c | 3 ++- src/lxc/lxc_driver.c | 6 ++++-- src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 9 ++++++--- src/qemu/qemu_driver.c | 6 ++++-- src/remote/remote_driver.c | 24 ++++++++++++++++-------- src/secret/secret_driver.c | 6 ++++-- src/storage/storage_driver.c | 3 ++- src/uml/uml_driver.c | 6 ++++-- 10 files changed, 48 insertions(+), 24 deletions(-)
ACK
Thanks, pushed.
How about adding ATTRIBUTE_RETURN_CHECK onto all the virRegister function declarations too, so gcc enforces this.
That's a good idea, I'll send another patch for that. Pavel
Regards, Daniel

On Mon, Mar 17, 2014 at 03:22:11PM +0100, Pavel Hrdina wrote:
Coverity found an issue in lxc_driver and uml_driver that we don't check the return value of register functions.
I've also updated all other places and unify the way we check the return value.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..c9711bb 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7825,15 +7825,23 @@ remoteRegister(void) { remoteDriver = &remote_driver;
- if (virRegisterDriver(&remote_driver) == -1) return -1; - if (virRegisterNetworkDriver(&network_driver) == -1) return -1; - if (virRegisterInterfaceDriver(&interface_driver) == -1) return -1; - if (virRegisterStorageDriver(&storage_driver) == -1) return -1; - if (virRegisterNodeDeviceDriver(&node_device_driver) == -1) return -1; - if (virRegisterSecretDriver(&secret_driver) == -1) return -1; - if (virRegisterNWFilterDriver(&nwfilter_driver) == -1) return -1; + if (virRegisterDriver(&remote_driver) < 0) + return -1; + if (virRegisterNetworkDriver(&network_driver) < 0) + return -1; + if (virRegisterInterfaceDriver(&interface_driver) < 0) + return -1; + if (virRegisterStorageDriver(&storage_driver) < 0) + return -1; + if (virRegisterNodeDeviceDriver(&node_device_driver) < 0) + return -1; + if (virRegisterSecretDriver(&secret_driver) < 0) + return -1; + if (virRegisterNWFilterDriver(&nwfilter_driver) < 0) + return -1; #ifdef WITH_LIBVIRTD - if (virRegisterStateDriver(&state_driver) == -1) return -1; + if (virRegisterStateDriver(&state_driver) == -1) < 0) + return -1; #endif
Opps, I missed that you left in the '== -1)' bit here Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange
-
Pavel Hrdina