[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak
From: |
Benoît Canet |
Subject: |
Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init() |
Date: |
Tue, 27 May 2014 23:43:12 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
The Tuesday 27 May 2014 à 21:44:15 (+0200), Markus Armbruster wrote :
> Benoît Canet <address@hidden> writes:
>
> > The Tuesday 27 May 2014 à 21:11:45 (+0200), Markus Armbruster wrote :
> >> Benoît Canet <address@hidden> writes:
> >>
> >> > The Tuesday 27 May 2014 à 18:13:12 (+0200), Markus Armbruster wrote :
> >> >> Benoît Canet <address@hidden> 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 <address@hidden>
> >> >> >> ---
> >> >> >> 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
>
> [...]
>
- Re: [Qemu-devel] [PATCH 07/14] qemu-io: Don't print NULL when open without non-option arg fails, (continued)
- [Qemu-devel] [PATCH 05/14] qemu-io: Support multiple -o in open command, Markus Armbruster, 2014/05/26
- [Qemu-devel] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Markus Armbruster, 2014/05/26
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Benoît Canet, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Markus Armbruster, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Benoît Canet, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Markus Armbruster, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Benoît Canet, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Markus Armbruster, 2014/05/27
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(),
Benoît Canet <=
- Re: [Qemu-devel] [Qemu-stable] [PATCH 09/14] blockdev: Plug memory leak in drive_init(), Markus Armbruster, 2014/05/28
[Qemu-devel] [PATCH 08/14] blockdev: Plug memory leak in blockdev_init(), Markus Armbruster, 2014/05/26