| Message ID | 1372929678-14341-1-git-send-email-mohan@in.ibm.com |
|---|---|
| State | New |
| Headers | show |
"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
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
Applied. Thanks. Regards, Anthony Liguori
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 */