From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35908) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xxls6-0003VG-KY for qemu-devel@nongnu.org; Sun, 07 Dec 2014 19:08:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xxls1-00049F-R3 for qemu-devel@nongnu.org; Sun, 07 Dec 2014 19:08:14 -0500 Received: from szxga02-in.huawei.com ([119.145.14.65]:5183) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xxls1-00048O-8C for qemu-devel@nongnu.org; Sun, 07 Dec 2014 19:08:09 -0500 Message-ID: <5484EBC7.4090804@huawei.com> Date: Mon, 8 Dec 2014 08:07:35 +0800 From: Gonglei MIME-Version: 1.0 References: <1417768210-9688-1-git-send-email-arei.gonglei@huawei.com> <5481AB9A.9020706@redhat.com> In-Reply-To: <5481AB9A.9020706@redhat.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] spice: fix memory leak List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: "pbonzini@redhat.com" , "Huangweidong (C)" , "qemu-devel@nongnu.org" , "kraxel@redhat.com" On 2014/12/5 20:56, Eric Blake wrote: > On 12/05/2014 01:30 AM, arei.gonglei@huawei.com wrote: >> From: Gonglei >> >> If errors happen for middle items of channel_list, >> qmp_query_spice_channels() returns NULL, and the variable >> cur_item going out of scope leaks the storage it points to. >> >> The flag is a compatibility thing for older spice-server >> versions. Meanwhile our minimum spice version requirement is >> new enough that we should never ever see this error, and if we >> do something went very seriously wrong. Let's using assert() >> instead of returning NULL to avoid a memory leak. >> >> Suggested-by: Paolo Bonzini >> Signed-off-by: Gonglei >> Reviewed-by: Eric Blake > > Putting in a R-b at the same time as fundamental changes (going from two > loops to an assert) is not something I would have done if submitting the > patch... > Yes, it's my fault :( >> --- >> v2: >> - using assert() instead of returning NULL. (Gerd) >> - fix some typos. (Eric) >> - add Eric's R-by tag, hope Eric has no objection >> because of changes of v2. :) > > ...but at least you caught my attention, and I am okay with v2, so you > can keep it now. > > Reviewed-by: Eric Blake > Thanks. Regards, -Gonglei