The Tuesday 27 May 2014 à 21:44:15 (+0200), Markus Armbruster wrote : > Benoît Canet <benoit.ca...@irqsave.net> writes: > > > 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." > > Well, before the patch, the leak exists, but there is no double-free. > > The patch fixes only one thing: the leak. It takes care not to break > things by freeing when it shouldn't. > > Do you still think the commit message should be amended? How?
Maybe just says it also take care not to introduce a double free. Best regards Benoît > > [...] >
