From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c1soz-0003Dk-IV for qemu-devel@nongnu.org; Wed, 02 Nov 2016 06:31:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c1sot-00043P-QE for qemu-devel@nongnu.org; Wed, 02 Nov 2016 06:31:05 -0400 Date: Wed, 2 Nov 2016 11:30:49 +0100 From: Kevin Wolf Message-ID: <20161102103049.GB6182@noname.redhat.com> References: <1478080120-3277-1-git-send-email-ashijeetacharya@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1478080120-3277-1-git-send-email-ashijeetacharya@gmail.com> Subject: Re: [Qemu-devel] [PATCH v2] block/nbd: Fix the regression to free leaked visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ashijeet Acharya Cc: mreitz@redhat.com, pbonzini@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org 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 > Reviewed-by: Eric Blake 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