[Spice-devel] [PATCH] fixup! RedChannelClient: store pipe items in a GQueue

[Jonathon Jongsma]

On Fri, 2016-09-16 at 10:05 +0100, Frediano Ziglio wrote:
> Minor changes:
> - use same name for dcc_add_surface_area_image argument in header
>   and source;
> - avoid using 2 variable in a for loop, is not much readable and
>   confusing. This also was fixing a regression quite hard to spot
>   so make sure code is less easy to break in the future;
> - remove check in red_drawable_pipe_item_free. Now
> RedDrawablePipeItem
>   is not forced to know in which container is it and the check should
>   be up to container. Also this was potentially O(n) expensive with
>   the GQueue implementation.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/dcc.c | 31 +++++++++++--------------------
>  server/dcc.h |  2 +-
>  2 files changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/server/dcc.c b/server/dcc.c
> index 184a944..cca3ce5 100644
> --- a/server/dcc.c
> +++ b/server/dcc.c
> @@ -68,7 +68,7 @@ int dcc_drawable_is_in_pipe(DisplayChannelClient
> *dcc, Drawable *drawable)
>  int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc,
> int surface_id,
>                                            int wait_if_used)
>  {
> -    GList *l, *item_pos = NULL;
> +    GList *l;
>      int x;
>      RedChannelClient *rcc;
>  
> @@ -77,13 +77,13 @@ int
> dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int
> surface
>         no other drawable depends on them */
>  
>      rcc = RED_CHANNEL_CLIENT(dcc);
> -    for (l = rcc->priv->pipe.head; l != NULL; item_pos = NULL) {
> +    for (l = rcc->priv->pipe.head; l != NULL; ) {
>          Drawable *drawable;
>          RedDrawablePipeItem *dpi = NULL;
>          int depend_found = FALSE;
>          RedPipeItem *item = l->data;
> +        GList *item_pos = l;
>  
> -        item_pos = l;
>          l = l->next;
>          if (item->type == RED_PIPE_ITEM_TYPE_DRAW) {
>              dpi = SPICE_CONTAINEROF(item, RedDrawablePipeItem,
> dpi_pipe_item);
> @@ -108,11 +108,11 @@ int
> dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int
> surface
>  
>          if (depend_found) {
>              spice_debug("surface %d dependent item found %p, %p",
> surface_id, drawable, item);
> -            if (wait_if_used) {
> -                break;
> -            } else {
> +            if (!wait_if_used) {
>                  return TRUE;
>              }
> +            return red_channel_client_wait_pipe_item_sent(rcc,
> item_pos,
> +                                                          COMMON_CLI
> ENT_TIMEOUT);
>          }
>      }
>  
> @@ -120,18 +120,11 @@ int
> dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int
> surface
>          return TRUE;
>      }
>  
> -    if (item_pos) {
> -        return
> red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc),
> item_pos,
> -                                                      COMMON_CLIENT_
> TIMEOUT);
> -    } else {
> -        /*
> -         * in case that the pipe didn't contain any item that is
> dependent on the surface, but
> -         * there is one during sending. Use a shorter timeout, since
> it is just one item
> -         */
> -        return
> red_channel_client_wait_outgoing_item(RED_CHANNEL_CLIENT(dcc),
> -                                                     DISPLAY_CLIENT_
> SHORT_TIMEOUT);
> -    }
> -    return TRUE;
> +    /*
> +     * in case that the pipe didn't contain any item that is
> dependent on the surface, but
> +     * there is one during sending. Use a shorter timeout, since it
> is just one item
> +     */
> +    return red_channel_client_wait_outgoing_item(rcc,
> DISPLAY_CLIENT_SHORT_TIMEOUT);
>  }
>  
>  void dcc_create_surface(DisplayChannelClient *dcc, int surface_id)
> @@ -284,8 +277,6 @@ static void
> red_drawable_pipe_item_free(RedPipeItem *item)
>                                                   dpi_pipe_item);
>      spice_assert(item->refcount == 0);
>  
> -    spice_warn_if_fail(!red_channel_client_pipe_item_is_linked(RED_C
> HANNEL_CLIENT(dpi->dcc),
> -                                                               &dpi-
> >dpi_pipe_item));
>      if (ring_item_is_linked(&dpi->base)) {
>          ring_remove(&dpi->base);
>      }
> diff --git a/server/dcc.h b/server/dcc.h
> index 7f93186..a2478b9 100644
> --- a/server/dcc.h
> +++ b/server/dcc.h
> @@ -129,7 +129,7 @@
> void                       dcc_push_surface_image                    
> (DisplayCha
>  RedImageItem
> *             dcc_add_surface_area_image                (DisplayChann
> elClient *dcc,
>                                                                      
>   int surface_id,
>                                                                      
>   SpiceRect *area,
> -                                                                    
>   GList *pos,
> +                                                                    
>   GList *pipe_item_pos,
>                                                                      
>   int can_lossy);
>  void                       dcc_palette_cache_reset                  
>  (DisplayChannelClient *dcc);
>  void                       dcc_palette_cache_palette                
>  (DisplayChannelClient *dcc,

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>