Re: [Qemu-devel] [PATCH v2] block/nbd: Fix the regression to free leaked visitor

[Kevin Wolf <kwolf@redhat.com>]

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 <ashijeetacharya@gmail.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

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