Re: [libvirt] [PATCH v2 12/32] storage: Use VIR_AUTOFREE for storage backends

On Fri, Feb 08, 2019 at 01:37:06PM -0500, John Ferlan wrote: > Let's make use of the auto __cleanup capabilities. This also allows > for the cleanup of some goto paths. > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > @@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, > void *data) > { > virStoragePoolSourceListPtr sourceList = data; > - char *pvname = NULL; > - char *vgname = NULL; > size_t i; > virStoragePoolSourceDevicePtr dev; > virStoragePoolSource *thisSource; > + VIR_AUTOFREE(char *) pvname = NULL; > + VIR_AUTOFREE(char *) vgname = NULL; > > if (VIR_STRDUP(pvname, groups[0]) < 0 || > VIR_STRDUP(vgname, groups[1]) < 0) > - goto error; > + return -1; > > thisSource = NULL; > for (i = 0; i < sourceList->nsources; i++) { > @@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, > > if (thisSource == NULL) { > if (!(thisSource = virStoragePoolSourceListNewSource(sourceList))) > - goto error; > + return -1; > > - thisSource->name = vgname; > + VIR_STEAL_PTR(thisSource->name, vgname); > } > - else > - VIR_FREE(vgname); > > if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0) > - goto error; > + return -1; > > dev = &thisSource->devices[thisSource->ndevice]; > thisSource->ndevice++; > thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2; > > memset(dev, 0, sizeof(*dev)); > - dev->path = pvname; > + VIR_STEAL_PTR(dev->path, pvname); I still don't see why ^this needs to stay and can't be separated into a preceding patch.

> > return 0; > - > - error: > - VIR_FREE(pvname); > - VIR_FREE(vgname); > - > - return -1; > } ... > > diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c > index f8bbde8cfe..7c2189d297 100644 > --- a/src/storage/storage_file_gluster.c > +++ b/src/storage/storage_file_gluster.c > @@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, > void *data) > { > virStorageFileBackendGlusterPrivPtr priv = data; > - char *buf = NULL; > size_t bufsiz = 0; > ssize_t ret; > struct stat st; > + VIR_AUTOFREE(char *) buf = NULL; > > *linkpath = NULL; > > @@ -277,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, > > realloc: > if (VIR_EXPAND_N(buf, bufsiz, 256) < 0) > - goto error; > + return -1; > > if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) { > virReportSystemError(errno, > _("failed to read link of gluster file '%s'"), > path); > - goto error; > + return -1; > } > > if (ret == bufsiz) > @@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, > > buf[ret] = '\0'; > > - *linkpath = buf; > + VIR_STEAL_PTR(*linkpath, buf); ^This VIR_STEAL_PTR can also be separated, it doesn't depend on the VIR_AUTOFREE stuff, it's a simple 1 change hunk.