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? [...]
