Re: [Xen-devel] [PATCH] x86/hap: Fix memory leak of domain->arch.hvm_domain.dirty_vram

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.11.12 at 02:00, Kouya Shimura <kouya@xxxxxxxxxxxxxx> wrote:
> On 11/28/2012 05:39 PM, Jan Beulich wrote:
>>>>> On 28.11.12 at 07:51, Kouya Shimura <kouya@xxxxxxxxxxxxxx> wrote:
>>> This patch fixes the memory leak of domain->arch.hvm_domain.dirty_vram.
>>>
>>> Signed-off-by: Kouya Shimura <kouya@xxxxxxxxxxxxxx>
>>
>> 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.
> 
>  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.

Tim will have the final say here anyway. But my general opinion
is that copying existing code is not an excuse to also copy its
eventual deficiencies.

> 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.

I didn't say you shouldn't clear the pointer field. All I said is
that the conditional around the freeing and clearing is pointless.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel