From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42617) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W5qih-0001TT-9J for qemu-devel@nongnu.org; Wed, 22 Jan 2014 00:51:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W5qic-0007uo-V4 for qemu-devel@nongnu.org; Wed, 22 Jan 2014 00:51:23 -0500 Message-ID: <52DF5C75.80505@redhat.com> Date: Wed, 22 Jan 2014 07:51:49 +0200 From: Orit Wasserman MIME-Version: 1.0 References: <33183CC9F5247A488A2544077AF19020815BB9E1@SZXEMA503-MBS.china.huawei.com> <52DE66E8.4040003@redhat.com> <33183CC9F5247A488A2544077AF19020815BBA0C@SZXEMA503-MBS.china.huawei.com> In-Reply-To: <33183CC9F5247A488A2544077AF19020815BBA0C@SZXEMA503-MBS.china.huawei.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] migration:fix free XBZRLE decoded_buf wrong List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" , "qemu-devel@nongnu.org" , "qemu-stable@nongnu.org" , Peter Maydell , "anthony@codemonkey.ws" , "pbonzini@redhat.com" Cc: "chenliang (T)" , Luonengjun , "Huangweidong (Hardware)" On 01/21/2014 02:58 PM, Gonglei (Arei) wrote: > >> -----Original Message----- >> From: Orit Wasserman [mailto:owasserm@redhat.com] >> Sent: Tuesday, January 21, 2014 8:24 PM >> To: Gonglei (Arei); qemu-devel@nongnu.org; qemu-stable@nongnu.org; Peter >> Maydell; anthony@codemonkey.ws; pbonzini@redhat.com >> Cc: chenliang (T); Luonengjun; Huangweidong (Hardware) >> Subject: Re: [Qemu-devel] [PATCH v2] migration:fix free XBZRLE decoded_buf >> wrong >> >> On 01/21/2014 02:11 PM, Gonglei (Arei) wrote: >>> Hi, >>> >>> This is an update of my patch. >>> Modifications in v2: >>> * Removing excess check for g_free >>> * The structure of XBZRLE is divided into two halves.One is for >>> * src side, another is for dest side. >>> >> >> What is the benefit of splitting the structure? >> decode_buf is only allocated (and freed) in the destination any way. > > Yeah, you are right. Splitting the structure is not necessary. > The key to do that is just for clear logic. As Peter said: > the current arrangement looks extremely prone to bugs like > this one where somebody forgets that some of the fields are > not relevant to whichever of src/dst the code path they're > writing is used on. > > Best regards, > -Gonglei > Sounds reasonable. Thanks for finding the leak and fixing it. Orit Orit