[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] block/nbd: Fix the regression to free leaked
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v2] block/nbd: Fix the regression to free leaked visitor |
Date: |
Wed, 2 Nov 2016 11:30:49 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 02.11.2016 um 10:48 hat Ashijeet Acharya geschrieben:
> This patch frees the leaked visitor in nbd_refresh_filename() and uses
> visit_free() to fix it. The leak was introduced by the commit 491d6c7.
>
> Signed-off-by: Ashijeet Acharya <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
I don't think this would generally be called a regression, so I'd change
the subject to just "nbd: Fix leaker visitor".
> Changes in v2:
> - Include the regression commit id in the commit message
> ---
> block/nbd.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 8ef1438..ff9d01a 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -545,6 +545,7 @@ static void nbd_refresh_filename(BlockDriverState *bs,
> QDict *options)
> qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid));
> }
>
> + visit_free(ov);
> qdict_flatten(opts);
> bs->full_open_options = opts;
> }
I would prefer freeing the visitor immediately after visit_complete() so
that everything visitor related is in a single place.
Both of these points don't make your patch wrong, of course, but would
you mind changing them?
Kevin