[Spice-devel] [PATCH 07/18] worker: simplify surface_update_dest()

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> ---
>  server/red_worker.c | 27 ++++++++++++++-------------
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 0a4b70c..3f7267a 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -938,21 +938,22 @@ static void image_surface_init(DisplayChannel *display)
>  
>  static void surface_update_dest(RedSurface *surface, const SpiceRect *area)
>  {
> -    if (!surface->context.canvas_draws_on_surface) {
> -        SpiceCanvas *canvas = surface->context.canvas;
> -        int h;
> -        int stride = surface->context.stride;
> -        uint8_t *line_0 = surface->context.line_0;
> +    SpiceCanvas *canvas = surface->context.canvas;
> +    int stride = surface->context.stride;
> +    uint8_t *line_0 = surface->context.line_0;
>  
> -        if (!(h = area->bottom - area->top)) {
> -            return;
> -        }
> +    if (surface->context.canvas_draws_on_surface)
> +        return;
>  
> -        spice_assert(stride < 0);
> -        uint8_t *dest = line_0 + (area->top * stride) + area->left *
> sizeof(uint32_t);
> -        dest += (h - 1) * stride;
> -        canvas->ops->read_bits(canvas, dest, -stride, area);
> -    }
> +    int h = area->bottom - area->top;

As you can see in this version h is computed after the canvas check.
The fact that this change remove the use after free scare me a bit.
Basically some area data are inside some red_drawable which get freed
while drawing.

> +    if (h == 0)
> +        return;
> +
> +    spice_return_if_fail(stride < 0);
> +
> +    uint8_t *dest = line_0 + (area->top * stride) + area->left *
> sizeof(uint32_t);
> +    dest += (h - 1) * stride;
> +    canvas->ops->read_bits(canvas, dest, -stride, area);
>  }
>  
>  /*

Frediano