[Spice-devel] [PATCH] dcc: free glz buffer after zlib compression

[Christophe Fergeau]

On Tue, Dec 22, 2015 at 07:26:42PM +0000, Frediano Ziglio wrote:
> For zlib+glz compression image if first compressed with glz then the
> resulting buffer is compressed again with zlib then the buffer from
> zlib is returned to the caller.
> However the temporary glz buffer was not freed resulting in a memory
> leak.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/dcc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index 7c16fc0..1a6a865 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -662,6 +662,9 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
>      int glz_size;
>      int zlib_size;
>  
> +    spice_assert(glz_data->data.bufs_tail == NULL);
> +    spice_assert(glz_data->data.bufs_head == NULL);
> +

I don't think this belongs to this patch, and this should be a
g_warn_if_fail() rather than an assertion. zlib_data would deserve the
same checks.

>      glz_data->data.bufs_tail = compress_buf_new();
>      glz_data->data.bufs_head = glz_data->data.bufs_tail;
>      glz_data->data.dcc = dcc;
> @@ -708,6 +711,13 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
>              compress_buf_free(buf);
>          }
>          goto glz;
> +    } else {
> +        while (glz_data->data.bufs_head) {
> +            RedCompressBuf *buf = glz_data->data.bufs_head;
> +            glz_data->data.bufs_head = buf->send_next;
> +            compress_buf_free(buf);
> +        }

This is repeated several times, imo this deserves to be moved to a
red_encoder_data_free() helper.

> +        glz_data->data.bufs_head = glz_data->data.bufs_tail = NULL;

glz_data->data.bufs_head should be NULL when we get here.



>      }
>  
>      dest->descriptor.type = SPICE_IMAGE_TYPE_ZLIB_GLZ_RGB;
> @@ -725,6 +735,7 @@ glz:
>  
>      o_comp_data->comp_buf = glz_data->data.bufs_head;
>      o_comp_data->comp_buf_size = glz_size;
> +    glz_data->data.bufs_head = glz_data->data.bufs_tail = NULL;

This seems unrelated to the patch?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20160106/59ab9a40/attachment.sig>