[libvirt] [PATCH 0/5] Resolve some Coverity issues
Recent changes to various modules seems to have awoken Coverity and caused perusal through new paths. Hopefully this sends OK as I had to reset my environment after a fairly massive disk failure. Of course restarting from scratch could be a factor in why Coverity found the new issues. John Ferlan (5): nwfilter: Remove Coverity DEADCODE warning virnetserverclient: Remove Coverity DEADCODE warning virnetserverclient: Fix conditional change HAVE_SASL to WITH_SASL bridge_driver: Resolve Coverity CHECKED_RETURN warning storage_driver: Resolve Coverity CHECKED_RETURN warning src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 2 -- src/rpc/virnetserverclient.c | 10 ++++++++-- src/storage/storage_driver.c | 3 ++- 4 files changed, 12 insertions(+), 6 deletions(-) -- 1.8.3.1
The nwfilterStateInitialize() would only assign sysbus inside a WITH_DBUS conditional, thus leavin a subsequent check for sysbus and nwfilterDriverInstallDBusMatches() as a no-op Rather than try to add WITH_DBUS conditions which ended up conflicting with the usage of HAVE_FIREWALLD conditionals, just remove the WITH_DBUS since virdbus.c has entry points for with and without conditions. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/nwfilter/nwfilter_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index d521adf..d21dd82 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -177,11 +177,9 @@ nwfilterStateInitialize(bool privileged, if (!privileged) return 0; -#if WITH_DBUS if (virDBusHasSystemBus() && !(sysbus = virDBusGetSystemBus())) return -1; -#endif /* WITH_DBUS */ if (VIR_ALLOC(driverState) < 0) return -1; -- 1.8.3.1
On 12/03/2013 07:18 AM, John Ferlan wrote:
The nwfilterStateInitialize() would only assign sysbus inside a WITH_DBUS conditional, thus leavin a subsequent check for sysbus
s/leavin/leaving/
and nwfilterDriverInstallDBusMatches() as a no-op
Rather than try to add WITH_DBUS conditions which ended up conflicting with the usage of HAVE_FIREWALLD conditionals, just remove the WITH_DBUS since virdbus.c has entry points for with and without conditions.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/nwfilter/nwfilter_driver.c | 2 -- 1 file changed, 2 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The x509dname is only set inside a WITH_GNUTLS conditional, so when used/check later on for NULL, Coverity detects this is not possible. Added WITH_GNUTLS around uses to remove message Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/rpc/virnetserverclient.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 0b9ab52..8acf914 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -658,7 +658,9 @@ virNetServerClientCreateIdentity(virNetServerClientPtr client) #if WITH_SASL char *saslname = NULL; #endif +#if WITH_GNUTLS char *x509dname = NULL; +#endif char *seccontext = NULL; virIdentityPtr ret = NULL; @@ -748,11 +750,13 @@ virNetServerClientCreateIdentity(virNetServerClientPtr client) saslname) < 0) goto error; #endif +#if WITH_GNUTLS if (x509dname && virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME, x509dname) < 0) goto error; +#endif if (seccontext && virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SELINUX_CONTEXT, @@ -770,7 +774,9 @@ cleanup: #if HAVE_SASL VIR_FREE(saslname); #endif +#if WITH_GNUTLS VIR_FREE(x509dname); +#endif return ret; error: -- 1.8.3.1
On 12/03/2013 07:18 AM, John Ferlan wrote:
The x509dname is only set inside a WITH_GNUTLS conditional, so when used/check later on for NULL, Coverity detects this is not possible. Added WITH_GNUTLS around uses to remove message
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/rpc/virnetserverclient.c | 6 ++++++ 1 file changed, 6 insertions(+)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/rpc/virnetserverclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 8acf914..8aebeb0 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -743,7 +743,7 @@ virNetServerClientCreateIdentity(virNetServerClientPtr client) VIR_IDENTITY_ATTR_UNIX_PROCESS_TIME, processtime) < 0) goto error; -#if HAVE_SASL +#if WITH_SASL if (saslname && virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SASL_USER_NAME, @@ -771,7 +771,7 @@ cleanup: VIR_FREE(processid); VIR_FREE(processtime); VIR_FREE(seccontext); -#if HAVE_SASL +#if WITH_SASL VIR_FREE(saslname); #endif #if WITH_GNUTLS -- 1.8.3.1
On 12/03/2013 07:18 AM, John Ferlan wrote:
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/rpc/virnetserverclient.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK.
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 8acf914..8aebeb0 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -743,7 +743,7 @@ virNetServerClientCreateIdentity(virNetServerClientPtr client) VIR_IDENTITY_ATTR_UNIX_PROCESS_TIME, processtime) < 0) goto error; -#if HAVE_SASL +#if WITH_SASL if (saslname && virIdentitySetAttr(ret, VIR_IDENTITY_ATTR_SASL_USER_NAME, @@ -771,7 +771,7 @@ cleanup: VIR_FREE(processid); VIR_FREE(processtime); VIR_FREE(seccontext); -#if HAVE_SASL +#if WITH_SASL VIR_FREE(saslname); #endif #if WITH_GNUTLS
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The networkRegister() didn't check the return status of the virRegisterNetworkDriver() call like other callers, so just check and handle here as well. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/network/bridge_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1e4cc70..80c5acb 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3046,7 +3046,8 @@ static virStateDriver networkStateDriver = { }; int networkRegister(void) { - virRegisterNetworkDriver(&networkDriver); + if (virRegisterNetworkDriver(&networkDriver) < 0) + return -1; virRegisterStateDriver(&networkStateDriver); return 0; } -- 1.8.3.1
On 12/03/2013 07:18 AM, John Ferlan wrote:
The networkRegister() didn't check the return status of the virRegisterNetworkDriver() call like other callers, so just check and handle here as well.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/network/bridge_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1e4cc70..80c5acb 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3046,7 +3046,8 @@ static virStateDriver networkStateDriver = { };
int networkRegister(void) { - virRegisterNetworkDriver(&networkDriver); + if (virRegisterNetworkDriver(&networkDriver) < 0) + return -1; virRegisterStateDriver(&networkStateDriver); return 0; }
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The storageRegister() didn't check the return from the virRegisterStorageDriver() like other callers did, so Coverity flagged it. Just check the return and handle. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 3b4715a..469d135 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2645,7 +2645,8 @@ static virStateDriver stateDriver = { }; int storageRegister(void) { - virRegisterStorageDriver(&storageDriver); + if (virRegisterStorageDriver(&storageDriver) < 0) + return -1; virRegisterStateDriver(&stateDriver); return 0; } -- 1.8.3.1
On 12/03/2013 07:18 AM, John Ferlan wrote:
The storageRegister() didn't check the return from the virRegisterStorageDriver() like other callers did, so Coverity flagged it. Just check the return and handle.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
ACK.
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 3b4715a..469d135 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2645,7 +2645,8 @@ static virStateDriver stateDriver = { };
int storageRegister(void) {
While touching this, you may want to fix the style to put the opening { on its own line.
- virRegisterStorageDriver(&storageDriver); + if (virRegisterStorageDriver(&storageDriver) < 0) + return -1; virRegisterStateDriver(&stateDriver); return 0; }
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12/03/2013 09:18 AM, John Ferlan wrote:
Recent changes to various modules seems to have awoken Coverity and caused perusal through new paths.
Hopefully this sends OK as I had to reset my environment after a fairly massive disk failure. Of course restarting from scratch could be a factor in why Coverity found the new issues.
John Ferlan (5): nwfilter: Remove Coverity DEADCODE warning virnetserverclient: Remove Coverity DEADCODE warning virnetserverclient: Fix conditional change HAVE_SASL to WITH_SASL bridge_driver: Resolve Coverity CHECKED_RETURN warning storage_driver: Resolve Coverity CHECKED_RETURN warning
src/network/bridge_driver.c | 3 ++- src/nwfilter/nwfilter_driver.c | 2 -- src/rpc/virnetserverclient.c | 10 ++++++++-- src/storage/storage_driver.c | 3 ++- 4 files changed, 12 insertions(+), 6 deletions(-)
Adjusted 1/5 and 5/5 and pushed. Thanks, John
participants (2)
-
Eric Blake -
John Ferlan