Message ID | 1371376960-18192-1-git-send-email-sw@weilnetz.de |
---|---|
State | Accepted |
Headers | show |
Am 16.06.2013 12:02, schrieb Stefan Weil: > This leak was reported by cppcheck. > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > fsdev/qemu-fsdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > index 6eaf36d..ccfec13 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts) > > if (fsle->fse.ops->parse_opts) { > if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) { > + g_free(fsle->fse.fsdev_id); > + g_free(fsle); > return -1; > } > } Ping?
Stefan Weil <sw@weilnetz.de> writes: > This leak was reported by cppcheck. > > Signed-off-by: Stefan Weil <sw@weilnetz.de> Reviewed-by: M. Mohan Kumar <mohan@in.ibm.com> > --- > fsdev/qemu-fsdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > index 6eaf36d..ccfec13 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts) > > if (fsle->fse.ops->parse_opts) { > if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) { > + g_free(fsle->fse.fsdev_id); > + g_free(fsle); > return -1; > } > } > -- > 1.7.10.4
Stefan Weil <sw@weilnetz.de> writes: > Am 16.06.2013 12:02, schrieb Stefan Weil: >> This leak was reported by cppcheck. >> >> Signed-off-by: Stefan Weil <sw@weilnetz.de> >> --- >> fsdev/qemu-fsdev.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c >> index 6eaf36d..ccfec13 100644 >> --- a/fsdev/qemu-fsdev.c >> +++ b/fsdev/qemu-fsdev.c >> @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts) >> >> if (fsle->fse.ops->parse_opts) { >> if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) { >> + g_free(fsle->fse.fsdev_id); >> + g_free(fsle); >> return -1; >> } >> } > > Ping? Both the error path result in Qemu terminating right ? Do we really care about free in those case ? -aneesh
Am 07.07.2013 19:15, schrieb Aneesh Kumar K.V: > Stefan Weil <sw@weilnetz.de> writes: > >> Am 16.06.2013 12:02, schrieb Stefan Weil: >>> This leak was reported by cppcheck. >>> >>> Signed-off-by: Stefan Weil <sw@weilnetz.de> >>> --- >>> fsdev/qemu-fsdev.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c >>> index 6eaf36d..ccfec13 100644 >>> --- a/fsdev/qemu-fsdev.c >>> +++ b/fsdev/qemu-fsdev.c >>> @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts) >>> >>> if (fsle->fse.ops->parse_opts) { >>> if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) { >>> + g_free(fsle->fse.fsdev_id); >>> + g_free(fsle); >>> return -1; >>> } >>> } >> Ping? > Both the error path result in Qemu terminating right ? Do we really care > about free in those case ? > > -aneesh Yes, we do, because it simplifies code reviews if there are as few as possible warnings from static code analysers. It is also not immediately obvious that QEMU terminates in the code which was patched here. Reviewers have to grep and look in vl.c, too, to see this. Therefore I'd appreciate if this and similar code could be fixed (with low priority here). Regards Stefan W.
Applied. Thanks. Regards, Anthony Liguori
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c index 6eaf36d..ccfec13 100644 --- a/fsdev/qemu-fsdev.c +++ b/fsdev/qemu-fsdev.c @@ -76,6 +76,8 @@ int qemu_fsdev_add(QemuOpts *opts) if (fsle->fse.ops->parse_opts) { if (fsle->fse.ops->parse_opts(opts, &fsle->fse)) { + g_free(fsle->fse.fsdev_id); + g_free(fsle); return -1; } }
This leak was reported by cppcheck. Signed-off-by: Stefan Weil <sw@weilnetz.de> --- fsdev/qemu-fsdev.c | 2 ++ 1 file changed, 2 insertions(+)