[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] spice: fix memory leak
From: |
Gonglei |
Subject: |
Re: [Qemu-devel] [PATCH v2] spice: fix memory leak |
Date: |
Mon, 8 Dec 2014 08:07:35 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 |
On 2014/12/5 20:56, Eric Blake wrote:
> On 12/05/2014 01:30 AM, address@hidden wrote:
>> From: Gonglei <address@hidden>
>>
>> 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 <address@hidden>
>> Signed-off-by: Gonglei <address@hidden>
>> Reviewed-by: Eric Blake <address@hidden>
>
> 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 <address@hidden>
>
Thanks.
Regards,
-Gonglei