On 09/21/2011 12:10 PM, ajia(a)redhat.com wrote: > From: Alex Jia<ajia(a)redhat.com> > > * src/locking/lock_driver_sanlock.c: in fact, virStrcpy calls > virStrncpy(dest, src, strlen(src), destbytes) then return result, > if 'path' is NULL, it means 'src' is also NULL, strlen(NULL) will > dereference a NULL pointer, which probably causes a segmentation > fault. > > Signed-off-by: Alex Jia<ajia(a)redhat.com> > --- > src/locking/lock_driver_sanlock.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/src/locking/lock_driver_sanlock.c > b/src/locking/lock_driver_sanlock.c > index 2d72510..8c6d873 100644 > --- a/src/locking/lock_driver_sanlock.c > +++ b/src/locking/lock_driver_sanlock.c > @@ -158,7 +158,7 @@ static int virLockManagerSanlockSetupLockspace(void) > memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, > SANLK_NAME_LEN); > ls.host_id = 0; /* Doesn't matter for initialization */ > ls.flags = 0; > - if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) { > + if (!path || !virStrcpy(ls.host_id_disk.path, path, > SANLK_PATH_LEN)) { > virLockError(VIR_ERR_INTERNAL_ERROR, > _("Lockspace path '%s' exceeded %d characters"), > path, SANLK_PATH_LEN); NACK. The prior virAsprintf guarantees that path is non-NULL at this point. Rather, the real problem that Coverity is complaining about here is that the only way to get to the error_unlink: label is if path is already non-NULL, so that the 'if (path)' in that label is redundant.

Detected by Coverity. The only way to get to error_unlink is if path was successfully assigned, so the if was useless. Meanwhile, there was a return statement that did not free path. * src/locking/lock_driver_sanlock.c (virLockManagerSanlockSetupLockspace): Fix mem-leak, and drop useless if. --- src/locking/lock_driver_sanlock.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 2d72510..13940f1 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -239,7 +239,7 @@ static int virLockManagerSanlockSetupLockspace(void) virReportSystemError(-rv, _("Unable to add lockspace %s"), path); - return -1; + goto error_unlink; } else { VIR_DEBUG("Lockspace %s is already registered", path); } @@ -250,8 +250,7 @@ static int virLockManagerSanlockSetupLockspace(void) return 0; error_unlink: - if (path) - unlink(path); + unlink(path); error: VIR_FORCE_CLOSE(fd); VIR_FREE(path);