[libvirt] [PATCH] storage: Fix memory leak in error path

[Jiri Denemark]

On Wed, Mar 06, 2013 at 07:35:15 -0500, John Ferlan wrote:
> On 03/06/2013 06:59 AM, Jiri Denemark wrote:
> > This also renames cleanup label as error since it is only used for error
> > path rather then being common for both success and error paths.
> > ---
> >  src/storage/storage_backend_logical.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
> > index bb709df..e3b3de4 100644
> > --- a/src/storage/storage_backend_logical.c
> > +++ b/src/storage/storage_backend_logical.c
> > @@ -741,10 +741,10 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> >          virCommandAddArg(cmd, pool->def->source.name);
> >  
> >      if (virCommandRun(cmd, NULL) < 0)
> > -        goto cleanup;
> > +        goto error;
> >  
> >      if ((fdret = virStorageBackendVolOpen(vol->target.path)) < 0)
> > -        goto cleanup;
> > +        goto error;
> >      fd = fdret;
> >  
> >      /* We can only chown/grp if root */
> > @@ -753,21 +753,21 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> >              virReportSystemError(errno,
> >                                   _("cannot set file owner '%s'"),
> >                                   vol->target.path);
> > -            goto cleanup;
> > +            goto error;
> >          }
> >      }
> >      if (fchmod(fd, vol->target.perms.mode) < 0) {
> >          virReportSystemError(errno,
> >                               _("cannot set file mode '%s'"),
> >                               vol->target.path);
> > -        goto cleanup;
> > +        goto error;
> >      }
> >  
> >      if (VIR_CLOSE(fd) < 0) {
> >          virReportSystemError(errno,
> >                               _("cannot close file '%s'"),
> >                               vol->target.path);
> > -        goto cleanup;
> > +        goto error;
> >      }
> >      fd = -1;
> >  
> > @@ -776,17 +776,18 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
> >          virReportSystemError(errno,
> >                               _("cannot find newly created volume '%s'"),
> >                               vol->target.path);
> > -        goto cleanup;
> > +        goto error;
> >      }
> >  
> >      return 0;
> 
> Would we be leaking cmd here?

Indeed. And there are few other places where this function is weird.
I'll send a v2 that cleans them all.

Jirka