6:43 a.m.
Perhaps due to other related changes in the code paths, my daily Coverity
scan uncovered a few new issues last week. It didn't seem any of the issues
found were critical path, so I waited for the 1.0.3 before posting.
I am running Coverity on a daily basis and trying to fine tune the "blame"
algorithm. Unfortunately when changes are made in sometimes related code
paths it allows Coverity to scan further thus sometimes uncovering some
paths that hadn't been covered before or had been blocked by other issues
Coverity uncovered in the path. I have a dialogue going with someone at
Coverity regarding these types of discoveries.
John Ferlan (3):
libxl_conf: Resolve Coverity issue with call to regcomp()
libxl_driver: Resolve Coverity errors
sheepdog: Add Coverity filter
src/libxl/libxl_conf.c | 11 ++++++++++-
src/libxl/libxl_driver.c | 10 ++++++----
src/storage/storage_backend_sheepdog.c | 2 ++
3 files changed, 18 insertions(+), 5 deletions(-)
--
1.8.1.2
6:43 a.m.
New subject: [libvirt] [PATCH 1/3] libxl_conf: Resolve Coverity issue with call to regcomp()
---
src/libxl/libxl_conf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 4ce5dec..b208dd8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -769,10 +769,19 @@ error:
virCapsPtr
libxlMakeCapabilities(libxl_ctx *ctx)
{
+ int err;
libxl_physinfo phy_info;
const libxl_version_info *ver_info;
- regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED);
+ err = regcomp(&xen_cap_rec, xen_cap_re, REG_EXTENDED);
+ if (err != 0) {
+ char error[100];
+ regerror(err, &xen_cap_rec, error, sizeof(error));
+ regfree(&xen_cap_rec);
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to compile regex %s"), error);
+ return NULL;
+ }
if (libxl_get_physinfo(ctx, &phy_info) != 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
--
1.8.1.2
10:53 a.m.
New subject: [libvirt] [PATCH 1/3] libxl_conf: Resolve Coverity issue with call to regcomp()
On 03/05/2013 05:43 AM, John Ferlan wrote:
---
src/libxl/libxl_conf.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:43 a.m.
New subject: [libvirt] [PATCH 2/3] libxl_driver: Resolve Coverity errors
1. The virObjectLock() call was unconditional, but Unlock was conditional
on vm being valid. Removed the check
2. A call to virDomainEventNewFromObj() isn't guaranteed to return an
event - that check needs to be made prior to libxlDomainEventQueue()
of the event. Did not add libxlDriverLock/Unlock around the call since
some callers already have lock taken
3. Need to initialize fd = -1 in libxlDoDomainSave() since we can jump
to cleanup before it's set.
4. Missing break;'s in libxlDomainModifyDeviceFlags() for case
LIBXL_DEVICE_UPDATE. The default: case would report an error
---
src/libxl/libxl_driver.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 22bd245..35c9e16 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -455,8 +455,7 @@ libxlAutostartDomain(virDomainObjPtr vm,
ret = 0;
cleanup:
- if (vm)
- virObjectUnlock(vm);
+ virObjectUnlock(vm);
return ret;
}
@@ -983,7 +982,8 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
restore_fd < 0 ?
VIR_DOMAIN_EVENT_STARTED_BOOTED :
VIR_DOMAIN_EVENT_STARTED_RESTORED);
- libxlDomainEventQueue(driver, event);
+ if (event)
+ libxlDomainEventQueue(driver, event);
libxl_domain_config_dispose(&d_config);
VIR_FREE(dom_xml);
@@ -2085,7 +2085,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
virDomainEventPtr event = NULL;
char *xml = NULL;
uint32_t xml_len;
- int fd;
+ int fd = -1;
int ret = -1;
if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
@@ -3561,6 +3561,7 @@ libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
break;
case LIBXL_DEVICE_UPDATE:
ret = libxlDomainUpdateDeviceConfig(vmdef, dev);
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown domain modify action %d"), action);
@@ -3585,6 +3586,7 @@ libxlDomainModifyDeviceFlags(virDomainPtr dom, const char *xml,
break;
case LIBXL_DEVICE_UPDATE:
ret = libxlDomainUpdateDeviceLive(priv, vm, dev);
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown domain modify action %d"), action);
--
1.8.1.2
11 a.m.
New subject: [libvirt] [PATCH 2/3] libxl_driver: Resolve Coverity errors
On 03/05/2013 05:43 AM, John Ferlan wrote:
1. The virObjectLock() call was unconditional, but Unlock was
conditional
on vm being valid. Removed the check
2. A call to virDomainEventNewFromObj() isn't guaranteed to return an
event - that check needs to be made prior to libxlDomainEventQueue()
of the event. Did not add libxlDriverLock/Unlock around the call since
some callers already have lock taken
Someday, we should remove the big libxl lock in the same way that danpb
just removed the big qemu lock; but that's an independent project.
3. Need to initialize fd = -1 in libxlDoDomainSave() since we can jump
to cleanup before it's set.
4. Missing break;'s in libxlDomainModifyDeviceFlags() for case
LIBXL_DEVICE_UPDATE. The default: case would report an error
More precisely, the device update fell through to the default case and
reported an error even if it had succeeded.
At any rate, all of these fixes look correct.
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:50 p.m.
New subject: [libvirt] [PATCH 2/3] libxl_driver: Resolve Coverity errors
Eric Blake wrote:
On 03/05/2013 05:43 AM, John Ferlan wrote:
> 1. The virObjectLock() call was unconditional, but Unlock was conditional
> on vm being valid. Removed the check
>
> 2. A call to virDomainEventNewFromObj() isn't guaranteed to return an
> event - that check needs to be made prior to libxlDomainEventQueue()
> of the event. Did not add libxlDriverLock/Unlock around the call since
> some callers already have lock taken
>
Someday, we should remove the big libxl lock in the same way that danpb
just removed the big qemu lock; but that's an independent project.
Agreed. I added that to the libxl todo list when I saw his excellent work.
Jim
6:43 a.m.
New subject: [libvirt] [PATCH 3/3] sheepdog: Add Coverity filter
Add a Coverity dead_error_condition tag since it's not possible for next
to be NULL at that point and thus exit the while loop. If next were NULL
the condition to set it would have done the return -1.
---
src/storage/storage_backend_sheepdog.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/storage/storage_backend_sheepdog.c
b/src/storage/storage_backend_sheepdog.c
index 218284d..7b564bd 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -85,6 +85,8 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
pool->available = pool->capacity - pool->allocation;
return 0;
+ /* coverity[dead_error_condition] - since we return -1 above we'll
+ * never get to return -1 below */
} while ((p = next));
return -1;
--
1.8.1.2
11:14 a.m.
New subject: [libvirt] [PATCH 3/3] sheepdog: Add Coverity filter
On 03/05/2013 05:43 AM, John Ferlan wrote:
Add a Coverity dead_error_condition tag since it's not possible
for next
to be NULL at that point and thus exit the while loop. If next were NULL
the condition to set it would have done the return -1.
---
src/storage/storage_backend_sheepdog.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/storage/storage_backend_sheepdog.c
b/src/storage/storage_backend_sheepdog.c
index 218284d..7b564bd 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -85,6 +85,8 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool,
pool->available = pool->capacity - pool->allocation;
return 0;
+ /* coverity[dead_error_condition] - since we return -1 above we'll
+ * never get to return -1 below */
} while ((p = next));
Instead of adding this fancy comment, why not just change the earlier
'return -1;' lines into 'break;'. Then the end of the function would be
reachable, we'd no longer have dead code, we don't need a
coverity-specific comment, and other code analyzers will not need
similar tricks at a later date.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:34 p.m.
On 03/05/2013 07:43 AM, John Ferlan wrote:
src/libxl/libxl_conf.c | 11 ++++++++++-
src/libxl/libxl_driver.c | 10 ++++++----
src/storage/storage_backend_sheepdog.c | 2 ++
3 files changed, 18 insertions(+), 5 deletions(-)
Changed sheepdog to use break and removed Coverity comments (now where
was my brain on that).
pushed the changes
John
4280
days inactive
4281
days old
8 comments
3 participants
participants (3)
-
Eric Blake -
Jim Fehlig -
John Ferlan