Benoît Canet <benoit.ca...@irqsave.net> writes: > The Tuesday 27 May 2014 à 18:13:12 (+0200), Markus Armbruster wrote : >> Benoît Canet <benoit.ca...@irqsave.net> writes: >> >> > The Monday 26 May 2014 à 19:37:10 (+0200), Markus Armbruster wrote : >> >> Introduced in commit f298d07. Spotted by Coverity. >> >> >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> >> --- >> >> blockdev.c | 2 ++ >> >> 1 file changed, 2 insertions(+) >> >> >> >> diff --git a/blockdev.c b/blockdev.c >> >> index 6460c70..7ec7d79 100644 >> >> --- a/blockdev.c >> >> +++ b/blockdev.c >> >> @@ -941,6 +941,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, >> >> BlockInterfaceType block_default_type) >> >> >> >> /* Actual block device init: Functionality shared with blockdev-add >> >> */ >> >> dinfo = blockdev_init(filename, bs_opts, &local_err); >> >> + bs_opts = NULL; > > What is the purpose of this line ? I though it was to avoid double unref.
Before this patch, bs_opts gets leaked on any path from its qdict_new() that doesn't go through blockdev_init(). The new line below frees it, but without the line above, it would free it a second time on paths that go through blockdev_init(). Clear now? >> >> if (dinfo == NULL) { >> >> if (local_err) { >> >> qerror_report_err(local_err); >> >> @@ -978,6 +979,7 @@ DriveInfo *drive_init(QemuOpts *all_opts, >> >> BlockInterfaceType block_default_type) >> >> >> >> fail: >> >> qemu_opts_del(legacy_opts); >> >> + QDECREF(bs_opts); >> >> return dinfo; >> >> } >> >> >> >> -- >> >> 1.9.3 >> >> >> >> >> > >> > This commits seems to fix two thing a leak and a double free. >> > Maybe reflecting this on the commit message would make the bs_opts = >> > NULL; clearer. >> >> I'm blind. Can you explain the double free to me?