From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kouya Shimura Subject: Re: [PATCH] x86/hap: Fix memory leak of domain->arch.hvm_domain.dirty_vram Date: Thu, 29 Nov 2012 10:00:26 +0900 Message-ID: <50B6B3AA.2080505@jp.fujitsu.com> References: <50B5B48C.7040804@jp.fujitsu.com> <50B5DBD702000078000AC021@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50B5DBD702000078000AC021@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 11/28/2012 05:39 PM, Jan Beulich wrote: >>>> On 28.11.12 at 07:51, Kouya Shimura wrote: >> This patch fixes the memory leak of domain->arch.hvm_domain.dirty_vram. >> >> Signed-off-by: Kouya Shimura > > Wouldn't it be more consistent (and less redundant) to do this > through calling hap_track_dirty_vram(d, 0, 0, ...)? And even if > not, the conditional around the freeing/clearing is pointless. > > Jan > From another point of view, it's consistent since it almost copied from shadow_teardown()@xen/arch/x86/mm/shadow/common.c. That is a sibling function of hap_teardown(). If it's not preferable, another cleanup patch should be made. IMHO, I feel no good to remain a pointer to the freed memory even though it never be dereferenced any more. So it makes sense to check the pointer and set it NULL. Thanks, Kouya