Re: [Libvir] Repository for work-in-progress storage patches

"Daniel P. Berrange" <berrange(a)redhat.com&gt; 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.