* [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path
@ 2013-07-04 9:21 M. Mohan Kumar
2013-07-07 17:18 ` Aneesh Kumar K.V
2013-07-10 19:33 ` Anthony Liguori
0 siblings, 2 replies; 4+ messages in thread
From: M. Mohan Kumar @ 2013-07-04 9:21 UTC (permalink / raw)
To: qemu-devel; +Cc: M. Mohan Kumar, aneesh.kumar
From: "M. Mohan Kumar" <mohan@in.ibm.com>
Fix few more memory leaks in virtio-9p-device.c detected using valgrind.
Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
---
hw/9pfs/virtio-9p-device.c | 26 +++++++++++++++++---------
1 file changed, 17 insertions(+), 9 deletions(-)
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index dc6f4e4..35e2af4 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -68,14 +68,14 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
"id = %s\n",
s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
- return -1;
+ goto out;
}
if (!s->fsconf.tag) {
/* we haven't specified a mount_tag */
fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
s->fsconf.fsdev_id);
- return -1;
+ goto out;
}
s->ctx.export_flags = fse->export_flags;
@@ -85,10 +85,10 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
if (len > MAX_TAG_LEN - 1) {
fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
"maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
- return -1;
+ goto out;
}
- s->tag = strdup(s->fsconf.tag);
+ s->tag = g_strdup(s->fsconf.tag);
s->ctx.uid = -1;
s->ops = fse->ops;
@@ -99,11 +99,11 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
if (s->ops->init(&s->ctx) < 0) {
fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
" and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
- return -1;
+ goto out;
}
if (v9fs_init_worker_threads() < 0) {
fprintf(stderr, "worker thread initialization failed\n");
- return -1;
+ goto out;
}
/*
@@ -115,18 +115,26 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
fprintf(stderr,
"error in converting name to path %s", strerror(errno));
- return -1;
+ goto out;
}
if (s->ops->lstat(&s->ctx, &path, &stat)) {
fprintf(stderr, "share path %s does not exist\n", fse->path);
- return -1;
+ goto out;
} else if (!S_ISDIR(stat.st_mode)) {
fprintf(stderr, "share path %s is not a directory\n", fse->path);
- return -1;
+ goto out;
}
v9fs_path_free(&path);
return 0;
+out:
+ g_free(s->ctx.fs_root);
+ g_free(s->tag);
+ virtio_cleanup(vdev);
+ v9fs_path_free(&path);
+
+ return -1;
+
}
/* virtio-9p device */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path
2013-07-04 9:21 [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path M. Mohan Kumar
@ 2013-07-07 17:18 ` Aneesh Kumar K.V
2013-07-07 17:31 ` Andreas Färber
2013-07-10 19:33 ` Anthony Liguori
1 sibling, 1 reply; 4+ messages in thread
From: Aneesh Kumar K.V @ 2013-07-07 17:18 UTC (permalink / raw)
To: M. Mohan Kumar, qemu-devel, Anthony Liguori
"M. Mohan Kumar" <mohan@in.ibm.com> writes:
> From: "M. Mohan Kumar" <mohan@in.ibm.com>
>
> Fix few more memory leaks in virtio-9p-device.c detected using valgrind.
>
> Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
> ---
> hw/9pfs/virtio-9p-device.c | 26 +++++++++++++++++---------
> 1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index dc6f4e4..35e2af4 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -68,14 +68,14 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
> fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
> "id = %s\n",
> s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
> - return -1;
> + goto out;
> }
>
> if (!s->fsconf.tag) {
> /* we haven't specified a mount_tag */
> fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
> s->fsconf.fsdev_id);
> - return -1;
> + goto out;
> }
>
> s->ctx.export_flags = fse->export_flags;
> @@ -85,10 +85,10 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
> if (len > MAX_TAG_LEN - 1) {
> fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
> "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
> - return -1;
> + goto out;
> }
>
> - s->tag = strdup(s->fsconf.tag);
> + s->tag = g_strdup(s->fsconf.tag);
> s->ctx.uid = -1;
>
> s->ops = fse->ops;
> @@ -99,11 +99,11 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
> if (s->ops->init(&s->ctx) < 0) {
> fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
> " and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
> - return -1;
> + goto out;
> }
> if (v9fs_init_worker_threads() < 0) {
> fprintf(stderr, "worker thread initialization failed\n");
> - return -1;
> + goto out;
> }
>
> /*
> @@ -115,18 +115,26 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
> if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
> fprintf(stderr,
> "error in converting name to path %s", strerror(errno));
> - return -1;
> + goto out;
> }
> if (s->ops->lstat(&s->ctx, &path, &stat)) {
> fprintf(stderr, "share path %s does not exist\n", fse->path);
> - return -1;
> + goto out;
> } else if (!S_ISDIR(stat.st_mode)) {
> fprintf(stderr, "share path %s is not a directory\n", fse->path);
> - return -1;
> + goto out;
> }
> v9fs_path_free(&path);
>
> return 0;
> +out:
> + g_free(s->ctx.fs_root);
> + g_free(s->tag);
> + virtio_cleanup(vdev);
> + v9fs_path_free(&path);
> +
> + return -1;
> +
> }
>
The error path will result in qemu terminating right ? Do we care about
g_free in those case ?
-aneesh
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path
2013-07-07 17:18 ` Aneesh Kumar K.V
@ 2013-07-07 17:31 ` Andreas Färber
0 siblings, 0 replies; 4+ messages in thread
From: Andreas Färber @ 2013-07-07 17:31 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Anthony Liguori, M. Mohan Kumar, qemu-devel
Am 07.07.2013 19:18, schrieb Aneesh Kumar K.V:
> "M. Mohan Kumar" <mohan@in.ibm.com> writes:
>
>> From: "M. Mohan Kumar" <mohan@in.ibm.com>
>>
>> Fix few more memory leaks in virtio-9p-device.c detected using valgrind.
>>
>> Signed-off-by: M. Mohan Kumar <mohan@in.ibm.com>
>> ---
>> hw/9pfs/virtio-9p-device.c | 26 +++++++++++++++++---------
>> 1 file changed, 17 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
>> index dc6f4e4..35e2af4 100644
>> --- a/hw/9pfs/virtio-9p-device.c
>> +++ b/hw/9pfs/virtio-9p-device.c
>> @@ -68,14 +68,14 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>> fprintf(stderr, "Virtio-9p device couldn't find fsdev with the "
>> "id = %s\n",
>> s->fsconf.fsdev_id ? s->fsconf.fsdev_id : "NULL");
>> - return -1;
>> + goto out;
>> }
>>
>> if (!s->fsconf.tag) {
>> /* we haven't specified a mount_tag */
>> fprintf(stderr, "fsdev with id %s needs mount_tag arguments\n",
>> s->fsconf.fsdev_id);
>> - return -1;
>> + goto out;
>> }
>>
>> s->ctx.export_flags = fse->export_flags;
>> @@ -85,10 +85,10 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>> if (len > MAX_TAG_LEN - 1) {
>> fprintf(stderr, "mount tag '%s' (%d bytes) is longer than "
>> "maximum (%d bytes)", s->fsconf.tag, len, MAX_TAG_LEN - 1);
>> - return -1;
>> + goto out;
>> }
>>
>> - s->tag = strdup(s->fsconf.tag);
>> + s->tag = g_strdup(s->fsconf.tag);
>> s->ctx.uid = -1;
>>
>> s->ops = fse->ops;
>> @@ -99,11 +99,11 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>> if (s->ops->init(&s->ctx) < 0) {
>> fprintf(stderr, "Virtio-9p Failed to initialize fs-driver with id:%s"
>> " and export path:%s\n", s->fsconf.fsdev_id, s->ctx.fs_root);
>> - return -1;
>> + goto out;
>> }
>> if (v9fs_init_worker_threads() < 0) {
>> fprintf(stderr, "worker thread initialization failed\n");
>> - return -1;
>> + goto out;
>> }
>>
>> /*
>> @@ -115,18 +115,26 @@ static int virtio_9p_device_init(VirtIODevice *vdev)
>> if (s->ops->name_to_path(&s->ctx, NULL, "/", &path) < 0) {
>> fprintf(stderr,
>> "error in converting name to path %s", strerror(errno));
>> - return -1;
>> + goto out;
>> }
>> if (s->ops->lstat(&s->ctx, &path, &stat)) {
>> fprintf(stderr, "share path %s does not exist\n", fse->path);
>> - return -1;
>> + goto out;
>> } else if (!S_ISDIR(stat.st_mode)) {
>> fprintf(stderr, "share path %s is not a directory\n", fse->path);
>> - return -1;
>> + goto out;
>> }
>> v9fs_path_free(&path);
>>
>> return 0;
>> +out:
>> + g_free(s->ctx.fs_root);
>> + g_free(s->tag);
>> + virtio_cleanup(vdev);
>> + v9fs_path_free(&path);
>> +
>> + return -1;
>> +
>> }
>>
>
> The error path will result in qemu terminating right ? Do we care about
> g_free in those case ?
See my patches on the list converting VirtioDevices to QOM realize:
Realization grows an Error **errp argument for reporting errors, and
it's up to the caller to decide whether to continue (think device_add)
or terminate (-device).
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path
2013-07-04 9:21 [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path M. Mohan Kumar
2013-07-07 17:18 ` Aneesh Kumar K.V
@ 2013-07-10 19:33 ` Anthony Liguori
1 sibling, 0 replies; 4+ messages in thread
From: Anthony Liguori @ 2013-07-10 19:33 UTC (permalink / raw)
To: M. Mohan Kumar, qemu-devel
Applied. Thanks.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-10 19:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-04 9:21 [Qemu-devel] [PATCH 1/1] hw/9pfs: Fix memory leak in error path M. Mohan Kumar
2013-07-07 17:18 ` Aneesh Kumar K.V
2013-07-07 17:31 ` Andreas Färber
2013-07-10 19:33 ` Anthony Liguori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).