[libvirt] [PATCH] storage: Fix memory leak in error path
John Ferlan
jferlan at redhat.com
Wed Mar 6 12:35:15 UTC 2013
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?
>
> - cleanup:
> + error:
> err = virSaveLastError();
> VIR_FORCE_CLOSE(fd);
> virStorageBackendLogicalDeleteVol(conn, pool, vol, 0);
> virCommandFree(cmd);
> virSetError(err);
> + virFreeError(err);
Not that it matters here, but I did find at least one path where a
virSaveLastError() didn't call virFreeError() (e.g in
storage_backend_logical.c).
> return -1;
> }
>
>
ACK otherwise.
John
More information about the libvir-list
mailing list