From 22fc6a48e65b75fbd1dc90c63bdb0df54c6b72e1 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 5 Jun 2020 16:33:21 +0100 Subject: [PATCH] red-channel-client: Change GQueue into a std::list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starts using smart pointers in the queue. Signed-off-by: Frediano Ziglio Acked-by: Julien Ropé --- server/dcc-send.cpp | 34 ++++--------- server/dcc.cpp | 19 +++---- server/dcc.h | 3 +- server/red-channel-client.cpp | 94 ++++++++++++++++++----------------- server/red-channel-client.h | 13 +++-- server/red-pipe-item.h | 2 + server/video-stream.cpp | 2 +- 7 files changed, 82 insertions(+), 85 deletions(-) diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp index 80ef2da3..687fd70f 100644 --- a/server/dcc-send.cpp +++ b/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(pipe_item)->drawable; + drawable = static_cast(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(pipe_item); - drawable = dpi->drawable; + auto dpi = static_cast(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); } } diff --git a/server/dcc.cpp b/server/dcc.cpp index c6fb3f61..d16da328 100644 --- a/server/dcc.cpp +++ b/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(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) diff --git a/server/dcc.h b/server/dcc.h index 3931686d..002da283 100644 --- a/server/dcc.h +++ b/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); diff --git a/server/red-channel-client.cpp b/server/red-channel-client.cpp index 017ff7e4..e8827911 100644 --- a/server/red-channel-client.cpp +++ b/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() { diff --git a/server/red-channel-client.h b/server/red-channel-client.h index 5092f2f0..69099882 100644 --- a/server/red-channel-client.h +++ b/server/red-channel-client.h @@ -18,12 +18,14 @@ #ifndef RED_CHANNEL_CLIENT_H_ #define RED_CHANNEL_CLIENT_H_ +#include #include #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> 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); diff --git a/server/red-pipe-item.h b/server/red-pipe-item.h index e0c5c630..e77f241b 100644 --- a/server/red-pipe-item.h +++ b/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 RedPipeItemPtr; + RedPipeItem *red_pipe_item_ref(RedPipeItem *item); void red_pipe_item_unref(RedPipeItem *item); diff --git a/server/video-stream.cpp b/server/video-stream.cpp index 578afae7..48deb7a9 100644 --- a/server/video-stream.cpp +++ b/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);