The Tuesday 27 May 2014 à 21:11:45 (+0200), Markus Armbruster wrote : > 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?
Clear from the start it fixes a potential double free. "This commits seems to fix two thing a leak and a double free." Best regards Benoît > > >> >> 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? >