Re: [Libvir] Repository for work-in-progress storage patches
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Sat, Jan 19, 2008 at 07:09:31PM +0100, Jim Meyering wrote:
...
> In storage_backend_loop.c, it looks like vol->target.path
can be leaked.
Which function is that in ? Since originally writing it I've changed all
error path cleanup code to simply call virStorageVolDefFree(), so there's
a central function which will free all members, rather than having cleanup
duplicated all over the place.
This is the state from yesterday:
static int virStorageBackendLoopVolCreate(virConnectPtr conn,
...
if ((vol->target.path = malloc(5 + strlen(vol->name) + 1)) == NULL) {
virStorageReportError(conn, VIR_ERR_NO_MEMORY, "target");
return -1;
}
strcpy(vol->target.path, "/dev/");
strcat(vol->target.path, vol->name);
if ((vol->key = strdup(vol->target.path)) == NULL) {
virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "storage vol key");
return -1;
}
if (virRun(conn, (char**)cmdargv, NULL) < 0)
return -1;
if ((fd = open64(vol->target.path, O_RDONLY)) < 0) {
virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "cannot read path
'%s': %d (%s)",
vol->target.path, errno, strerror(errno));
goto cleanup;
}
...
cleanup:
close(fd);
virStorageBackendLoopVolDelete(conn, pool, vol);
return -1;
}
-----------------------------
I'd say just do s/return -1/goto cleanup/ after failed strdup
but then you have to be careful to set fd = -1, and avoid calling
"close" on it in that case.