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

[Frediano Ziglio]

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_CLIENT_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_CHANNEL_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                (DisplayChannelClient *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,
-- 
2.7.4