Re: [Qemu-devel] [PATCH v2] spice: fix memory leak

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

On 12/05/2014 01:30 AM, arei.gonglei@huawei.com wrote:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> 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 <pbonzini@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

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

> ---
> 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 <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]