red-channel-client: Change GQueue into a std::list

Starts using smart pointers in the queue.

Signed-off-by: Frediano Ziglio <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>

server/dcc-send.cpp

@@ -172,11 +172,6 @@ static bool is_brush_lossy(DisplayChannelClient *dcc, SpiceBrush *brush,
     }
 }
 
-static GList *dcc_get_tail(DisplayChannelClient *dcc)
-{
-    return dcc->get_pipe()->tail;
-}
-
 static void red_display_add_image_to_pixmap_cache(DisplayChannelClient *dcc,
                                                   SpiceImage *image, SpiceImage *io_image,
                                                   int is_lossy)
@@ -616,17 +611,14 @@ static bool pipe_rendered_drawables_intersect_with_areas(DisplayChannelClient *d
                                                          SpiceRect *surface_areas[],
                                                          int num_surfaces)
 {
-    GList *l;
-
     spice_assert(num_surfaces);
 
-    for (l = dcc->get_pipe()->head; l != NULL; l = l->next) {
+    for (const auto &pipe_item : dcc->get_pipe()) {
         Drawable *drawable;
-        RedPipeItem *pipe_item = (RedPipeItem *) l->data;
 
         if (pipe_item->type != RED_PIPE_ITEM_TYPE_DRAW)
             continue;
-        drawable = static_cast<RedDrawablePipeItem*>(pipe_item)->drawable;
+        drawable = static_cast<RedDrawablePipeItem*>(pipe_item.get())->drawable;
 
         if (ring_item_is_linked(&drawable->list_link))
             continue; // item hasn't been rendered
@@ -701,26 +693,22 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
     int resent_surface_ids[MAX_PIPE_SIZE];
     SpiceRect resent_areas[MAX_PIPE_SIZE]; // not pointers since drawables may be released
     int num_resent;
-    GList *l, *prev;
-    GQueue *pipe;
 
     resent_surface_ids[0] = first_surface_id;
     resent_areas[0] = *first_area;
     num_resent = 1;
 
-    pipe = dcc->get_pipe();
+    auto &pipe = dcc->get_pipe();
 
     // going from the oldest to the newest
-    for (l = pipe->tail; l != NULL; l = prev) {
-        RedPipeItem *pipe_item = (RedPipeItem *) l->data;
-        Drawable *drawable;
-        RedDrawablePipeItem *dpi;
+    for (auto l = pipe.end(); l != pipe.begin(); ) {
+        --l;
+        RedPipeItem *pipe_item = l->get();
 
-        prev = l->prev;
         if (pipe_item->type != RED_PIPE_ITEM_TYPE_DRAW)
             continue;
-        dpi = static_cast<RedDrawablePipeItem*>(pipe_item);
-        drawable = dpi->drawable;
+        auto dpi = static_cast<RedDrawablePipeItem*>(pipe_item);
+        auto drawable = dpi->drawable;
         if (ring_item_is_linked(&drawable->list_link))
             continue; // item hasn't been rendered
 
@@ -741,7 +729,7 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
         resent_areas[num_resent] = drawable->red_drawable->bbox;
         num_resent++;
 
-        dcc->pipe_remove_and_release_pos(l);
+        l = pipe.erase(l);
     }
 }
 
@@ -788,7 +776,7 @@ static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
         // will be executed before the current drawable
         for (i = 0; i < num_deps; i++) {
             dcc_add_surface_area_image(dcc, deps_surfaces_ids[i], deps_areas[i],
-                                       dcc_get_tail(dcc), FALSE);
+                                       dcc->get_pipe().end(), FALSE);
 
         }
     } else {
@@ -809,7 +797,7 @@ static void red_add_lossless_drawable_dependencies(DisplayChannelClient *dcc,
         }
 
         dcc_add_surface_area_image(dcc, drawable->surface_id, &drawable->bbox,
-                                   dcc_get_tail(dcc), TRUE);
+                                   dcc->get_pipe().end(), TRUE);
     }
 }
 

server/dcc.cpp

@@ -106,21 +106,21 @@ bool dcc_drawable_is_in_pipe(DisplayChannelClient *dcc, Drawable *drawable)
 bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface_id,
                                            int wait_if_used)
 {
-    GList *l;
     int x;
 
     spice_return_val_if_fail(dcc != NULL, TRUE);
     /* removing the newest drawables that their destination is surface_id and
        no other drawable depends on them */
 
-    for (l = dcc->get_pipe()->head; l != NULL; ) {
+    auto &pipe = dcc->get_pipe();
+    for (auto l = pipe.begin(); l != pipe.end(); ) {
         Drawable *drawable;
         RedDrawablePipeItem *dpi = NULL;
         int depend_found = FALSE;
-        RedPipeItem *item = (RedPipeItem *) l->data;
-        GList *item_pos = l;
+        RedPipeItem *item = l->get();
+        auto item_pos = l;
 
-        l = l->next;
+        ++l;
         if (item->type == RED_PIPE_ITEM_TYPE_DRAW) {
             dpi = static_cast<RedDrawablePipeItem*>(item);
             drawable = dpi->drawable;
@@ -131,7 +131,7 @@ bool dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surfac
         }
 
         if (drawable->surface_id == surface_id) {
-            dcc->pipe_remove_and_release_pos(item_pos);
+            l = pipe.erase(item_pos);
             continue;
         }
 
@@ -197,7 +197,8 @@ RedImageItem::RedImageItem():
 // adding the pipe item after pos. If pos == NULL, adding to head.
 void
 dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
-                           SpiceRect *area, GList *pipe_item_pos, int can_lossy)
+                           SpiceRect *area, RedChannelClient::Pipe::iterator pipe_item_pos,
+                           int can_lossy)
 {
     DisplayChannel *display = DCC_TO_DC(dcc);
     RedSurface *surface = &display->priv->surfaces[surface_id];
@@ -244,7 +245,7 @@ dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
         }
     }
 
-    if (pipe_item_pos) {
+    if (pipe_item_pos != dcc->get_pipe().end()) {
         dcc->pipe_add_after_pos(item, pipe_item_pos);
     } else {
         dcc->pipe_add(item);
@@ -272,7 +273,7 @@ void dcc_push_surface_image(DisplayChannelClient *dcc, int surface_id)
 
     /* not allowing lossy compression because probably, especially if it is a primary surface,
        it combines both "picture-like" areas with areas that are more "artificial"*/
-    dcc_add_surface_area_image(dcc, surface_id, &area, NULL, FALSE);
+    dcc_add_surface_area_image(dcc, surface_id, &area, dcc->get_pipe().end(), FALSE);
 }
 
 static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *drawable)

server/dcc.h

@@ -186,7 +186,8 @@ int                        dcc_compress_image                        (DisplayCha
                                                                       compress_send_data_t* o_comp_data);
 
 void dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
-                                SpiceRect *area, GList *pipe_item_pos, int can_lossy);
+                                SpiceRect *area, RedChannelClient::Pipe::iterator pipe_item_pos,
+                                int can_lossy);
 VideoStreamAgent *dcc_get_video_stream_agent(DisplayChannelClient *dcc, int stream_id);
 ImageEncoders *dcc_get_encoders(DisplayChannelClient *dcc);
 spice_wan_compression_t    dcc_get_jpeg_state                        (DisplayChannelClient *dcc);

server/red-channel-client.cpp

@@ -156,7 +156,7 @@ struct RedChannelClientPrivate
 
     bool block_read;
     bool during_send;
-    GQueue pipe;
+    RedChannelClient::Pipe pipe;
 
     RedChannelCapabilities remote_caps;
     bool is_mini_header;
@@ -173,7 +173,7 @@ struct RedChannelClientPrivate
     RedStatCounter out_messages;
     RedStatCounter out_bytes;
 
-    inline RedPipeItem *pipe_item_get();
+    inline RedPipeItemPtr pipe_item_get();
     inline bool pipe_remove(RedPipeItem *item);
     void handle_pong(SpiceMsgPing *ping);
     inline void set_message_serial(uint64_t serial);
@@ -307,8 +307,6 @@ RedChannelClientPrivate::RedChannelClientPrivate(RedChannel *init_channel,
 
     send_data.marshaller = send_data.main.marshaller;
 
-    g_queue_init(&pipe);
-
     red_channel_capabilities_reset(&remote_caps);
     red_channel_capabilities_init(&remote_caps, caps);
 
@@ -550,7 +548,7 @@ void RedChannelClient::msg_sent()
         spice_assert(priv->send_data.header.data != NULL);
         begin_send_message();
     } else {
-        if (g_queue_is_empty(&priv->pipe)) {
+        if (priv->pipe.empty()) {
             /* It is possible that the socket will become idle, so we may be able to test latency */
             priv->restart_ping_timer();
         }
@@ -558,9 +556,23 @@ void RedChannelClient::msg_sent()
 
 }
 
+static RedChannelClient::Pipe::iterator
+find_pipe_item(RedChannelClient::Pipe &pipe, const RedPipeItem *item)
+{
+    return std::find_if(pipe.begin(), pipe.end(),
+                        [=](const RedPipeItemPtr& p) -> bool {
+                            return p.get() == item;
+    });
+}
+
 bool RedChannelClientPrivate::pipe_remove(RedPipeItem *item)
 {
-    return g_queue_remove(&pipe, item);
+    auto i = find_pipe_item(pipe, item);
+    if (i != pipe.end()) {
+        pipe.erase(i);
+        return true;
+    }
+    return false;
 }
 
 bool RedChannelClient::test_remote_common_cap(uint32_t cap) const
@@ -1111,17 +1123,21 @@ void RedChannelClient::send()
     handle_outgoing();
 }
 
-inline RedPipeItem *RedChannelClientPrivate::pipe_item_get()
+inline RedPipeItemPtr RedChannelClientPrivate::pipe_item_get()
 {
-    if (send_data.blocked || waiting_for_ack()) {
-        return NULL;
+    RedPipeItemPtr ret;
+
+    if (send_data.blocked || waiting_for_ack() || pipe.empty()) {
+        return ret;
     }
-    return (RedPipeItem*) g_queue_pop_tail(&pipe);
+    ret = std::move(pipe.back());
+    pipe.pop_back();
+    return ret;
 }
 
 void RedChannelClient::push()
 {
-    RedPipeItem *pipe_item;
+    RedPipeItemPtr pipe_item;
 
     if (priv->during_send) {
         return;
@@ -1140,7 +1156,7 @@ void RedChannelClient::push()
     }
 
     while ((pipe_item = priv->pipe_item_get())) {
-        send_any_item(pipe_item);
+        send_any_item(pipe_item.get());
     }
     /* prepare_pipe_add() will reenable WRITE events when the priv->pipe is empty
      * ack_zero_messages_window() will reenable WRITE events
@@ -1149,7 +1165,7 @@ void RedChannelClient::push()
      * notified that we can write and we then exit (see pipe_item_get) as we
      * are waiting for the ack consuming CPU in a tight loop
      */
-    if ((no_item_being_sent() && g_queue_is_empty(&priv->pipe)) ||
+    if ((no_item_being_sent() && priv->pipe.empty()) ||
         priv->waiting_for_ack()) {
         priv->watch_update_mask(SPICE_WATCH_EVENT_READ);
 
@@ -1357,7 +1373,7 @@ inline bool RedChannelClient::prepare_pipe_add(RedPipeItem *item)
         red_pipe_item_unref(item);
         return false;
     }
-    if (g_queue_is_empty(&priv->pipe)) {
+    if (priv->pipe.empty()) {
         priv->watch_update_mask(SPICE_WATCH_EVENT_READ | SPICE_WATCH_EVENT_WRITE);
     }
     return true;
@@ -1365,11 +1381,10 @@ inline bool RedChannelClient::prepare_pipe_add(RedPipeItem *item)
 
 void RedChannelClient::pipe_add(RedPipeItem *item)
 {
-
     if (!prepare_pipe_add(item)) {
         return;
     }
-    g_queue_push_head(&priv->pipe, item);
+    priv->pipe.push_front(RedPipeItemPtr(item));
 }
 
 void RedChannelClient::pipe_add_push(RedPipeItem *item)
@@ -1379,41 +1394,40 @@ void RedChannelClient::pipe_add_push(RedPipeItem *item)
 }
 
 void RedChannelClient::pipe_add_after_pos(RedPipeItem *item,
-                                          GList *pipe_item_pos)
+                                          Pipe::iterator pipe_item_pos)
 {
-    spice_assert(pipe_item_pos);
+    spice_assert(pipe_item_pos != priv->pipe.end());
     if (!prepare_pipe_add(item)) {
         return;
     }
 
-    g_queue_insert_after(&priv->pipe, pipe_item_pos, item);
+    ++pipe_item_pos;
+    priv->pipe.insert(pipe_item_pos, RedPipeItemPtr(item));
 }
 
 void
-RedChannelClient::pipe_add_before_pos(RedPipeItem *item, GList *pipe_item_pos)
+RedChannelClient::pipe_add_before_pos(RedPipeItem *item, Pipe::iterator pipe_item_pos)
 {
-    spice_assert(pipe_item_pos);
+    spice_assert(pipe_item_pos != priv->pipe.end());
     if (!prepare_pipe_add(item)) {
         return;
     }
 
-    g_queue_insert_before(&priv->pipe, pipe_item_pos, item);
+    priv->pipe.insert(pipe_item_pos, RedPipeItemPtr(item));
 }
 
 void RedChannelClient::pipe_add_after(RedPipeItem *item, RedPipeItem *pos)
 {
-    GList *prev;
-
     spice_assert(pos);
-    prev = g_queue_find(&priv->pipe, pos);
-    g_return_if_fail(prev != NULL);
+    auto prev = find_pipe_item(priv->pipe, pos);
+    g_return_if_fail(prev != priv->pipe.end());
 
     pipe_add_after_pos(item, prev);
 }
 
 int RedChannelClient::pipe_item_is_linked(RedPipeItem *item)
 {
-    return g_queue_find(&priv->pipe, item) != NULL;
+    return find_pipe_item(priv->pipe, item) != priv->pipe.end();
 }
 
 void RedChannelClient::pipe_add_tail(RedPipeItem *item)
@@ -1421,7 +1435,7 @@ void RedChannelClient::pipe_add_tail(RedPipeItem *item)
     if (!prepare_pipe_add(item)) {
         return;
     }
-    g_queue_push_tail(&priv->pipe, item);
+    priv->pipe.push_back(RedPipeItemPtr(item));
 }
 
 void RedChannelClient::pipe_add_type(int pipe_item_type)
@@ -1446,17 +1460,17 @@ void RedChannelClient::pipe_add_empty_msg(int msg_type)
 
 gboolean RedChannelClient::pipe_is_empty()
 {
-    return g_queue_is_empty(&priv->pipe);
+    return priv->pipe.empty();
 }
 
 uint32_t RedChannelClient::get_pipe_size()
 {
-    return g_queue_get_length(&priv->pipe);
+    return priv->pipe.size();
 }
 
-GQueue* RedChannelClient::get_pipe()
+RedChannelClient::Pipe& RedChannelClient::get_pipe()
 {
-    return &priv->pipe;
+    return priv->pipe;
 }
 
 bool RedChannelClient::is_mini_header() const
@@ -1481,12 +1495,8 @@ void RedChannelClientPrivate::clear_sent_item()
 // are we reading from an fd here? arghh
 void RedChannelClientPrivate::pipe_clear()
 {
-    RedPipeItem *item;
-
     clear_sent_item();
-    while ((item = (RedPipeItem*) g_queue_pop_head(&pipe)) != NULL) {
-        red_pipe_item_unref(item);
-    }
+    pipe.clear();
 }
 
 void RedChannelClient::ack_zero_messages_window()
@@ -1561,7 +1571,7 @@ void RedChannelClient::set_header_sub_list(uint32_t sub_list)
 }
 
 /* TODO: more evil sync stuff. anything with the word wait in it's name. */
-bool RedChannelClient::wait_pipe_item_sent(GList *item_pos, int64_t timeout)
+bool RedChannelClient::wait_pipe_item_sent(Pipe::iterator item_pos, int64_t timeout)
 {
     uint64_t end_time;
 
@@ -1638,14 +1648,6 @@ void RedChannelClient::pipe_remove_and_release(RedPipeItem *item)
     }
 }
 
-void RedChannelClient::pipe_remove_and_release_pos(GList *item_pos)
-{
-    RedPipeItem *item = (RedPipeItem*) item_pos->data;
-
-    g_queue_delete_link(&priv->pipe, item_pos);
-    red_pipe_item_unref(item);
-}
-
 /* client mutex should be locked before this call */
 bool RedChannelClient::set_migration_seamless()
 {

server/red-channel-client.h

@@ -18,12 +18,14 @@
 #ifndef RED_CHANNEL_CLIENT_H_
 #define RED_CHANNEL_CLIENT_H_
 
+#include <list>
 #include <common/marshaller.h>
 
 #include "red-pipe-item.h"
 #include "red-stream.h"
 #include "red-channel.h"
 #include "utils.hpp"
+#include "safe-list.hpp"
 
 #include "push-visibility.h"
 
@@ -81,13 +83,14 @@ protected:
     void start_connectivity_monitoring(uint32_t timeout_ms);
 
 public:
+    typedef std::list<RedPipeItemPtr, red::Mallocator<RedPipeItemPtr>> Pipe;
+
     void pipe_add_push(RedPipeItem *item);
     void pipe_add(RedPipeItem *item);
     void pipe_add_after(RedPipeItem *item, RedPipeItem *pos);
-    void pipe_add_after_pos(RedPipeItem *item, GList *pos);
+    void pipe_add_after_pos(RedPipeItem *item, RedChannelClient::Pipe::iterator pos);
     int pipe_item_is_linked(RedPipeItem *item);
     void pipe_remove_and_release(RedPipeItem *item);
-    void pipe_remove_and_release_pos(GList *item_pos);
     void pipe_add_tail(RedPipeItem *item);
     /* for types that use this routine -> the pipe item should be freed */
     void pipe_add_type(int pipe_item_type);
@@ -95,7 +98,7 @@ public:
     void pipe_add_empty_msg(int msg_type);
     gboolean pipe_is_empty();
     uint32_t get_pipe_size();
-    GQueue* get_pipe();
+    Pipe& get_pipe();
     bool is_mini_header() const;
 
     void ack_zero_messages_window();
@@ -130,7 +133,7 @@ public:
      * Return: TRUE if waiting succeeded. FALSE if timeout expired.
      */
 
-    bool wait_pipe_item_sent(GList *item_pos, int64_t timeout);
+    bool wait_pipe_item_sent(RedChannelClient::Pipe::iterator item_pos, int64_t timeout);
     bool wait_outgoing_item(int64_t timeout);
 
     RedChannel* get_channel();
@@ -181,7 +184,7 @@ private:
     virtual void handle_migrate_flush_mark();
     void handle_migrate_data_early(uint32_t size, void *message);
     inline bool prepare_pipe_add(RedPipeItem *item);
-    void pipe_add_before_pos(RedPipeItem *item, GList *pipe_item_pos);
+    void pipe_add_before_pos(RedPipeItem *item, RedChannelClient::Pipe::iterator pipe_item_pos);
     void send_set_ack();
     void send_migrate();
     void send_empty_msg(RedPipeItem *base);

server/red-pipe-item.h

@@ -64,6 +64,8 @@ struct RedPipeItem: public red::shared_ptr_counted
     void add_to_marshaller(SpiceMarshaller *m, uint8_t *data, size_t size);
 };
 
+typedef red::shared_ptr<RedPipeItem> RedPipeItemPtr;
+
 RedPipeItem *red_pipe_item_ref(RedPipeItem *item);
 void red_pipe_item_unref(RedPipeItem *item);
 

server/video-stream.cpp

@@ -864,7 +864,7 @@ static void dcc_detach_stream_gracefully(DisplayChannelClient *dcc,
         } else {
             display_channel_draw(display, &upgrade_area, 0);
         }
-        dcc_add_surface_area_image(dcc, 0, &upgrade_area, NULL, FALSE);
+        dcc_add_surface_area_image(dcc, 0, &upgrade_area, dcc->get_pipe().end(), FALSE);
     }
 clear_vis_region:
     region_clear(&agent->vis_region);