From 45e964dc5a2e2f86437da0ef3d31ddb650109f40 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Fri, 5 Jun 2020 14:03:35 +0100 Subject: [PATCH] red-pipe-item: Better encapsulate marshaller_unref_pipe_item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To avoid memory errors marshaller_unref_pipe_item should be matched with a previous call to red_pipe_item_ref. This is correctly done but to reduce the possibility of breaking this rule move both referencing and unreferencing into a new RedPipeItem::add_to_marshaller method. Signed-off-by: Frediano Ziglio Acked-by: Julien RopĂ© --- server/cursor-channel.cpp | 4 +--- server/dcc-send.cpp | 6 ++---- server/main-channel-client.cpp | 3 +-- server/red-pipe-item.cpp | 9 ++++++++- server/red-pipe-item.h | 11 ++++++----- server/smartcard-channel-client.cpp | 6 ++---- server/spicevmc.cpp | 4 +--- server/stream-channel.cpp | 4 +--- 8 files changed, 22 insertions(+), 25 deletions(-) diff --git a/server/cursor-channel.cpp b/server/cursor-channel.cpp index 05b7228a..835597f4 100644 --- a/server/cursor-channel.cpp +++ b/server/cursor-channel.cpp @@ -89,9 +89,7 @@ static void cursor_fill(CursorChannelClient *ccc, RedCursorPipeItem *cursor, if (red_cursor->data_size) { SpiceMarshaller *m2 = spice_marshaller_get_submarshaller(m); - red_pipe_item_ref(&cursor->base); - spice_marshaller_add_by_ref_full(m2, red_cursor->data, red_cursor->data_size, - marshaller_unref_pipe_item, &cursor->base); + cursor->base.add_to_marshaller(m2, red_cursor->data, red_cursor->data_size); } } diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp index 727507fb..7c070f3a 100644 --- a/server/dcc-send.cpp +++ b/server/dcc-send.cpp @@ -1969,10 +1969,8 @@ static void red_marshall_image(DisplayChannelClient *dcc, spice_marshall_Image(src_bitmap_out, &red_image, &bitmap_palette_out, &lzplt_palette_out); - red_pipe_item_ref(&item->base); - spice_marshaller_add_by_ref_full(src_bitmap_out, item->data, - bitmap.y * bitmap.stride, - marshaller_unref_pipe_item, item); + item->base.add_to_marshaller(src_bitmap_out, item->data, + bitmap.y * bitmap.stride); region_remove(surface_lossy_region, ©.base.box); } spice_chunks_destroy(chunks); diff --git a/server/main-channel-client.cpp b/server/main-channel-client.cpp index 6c0a8200..90df5704 100644 --- a/server/main-channel-client.cpp +++ b/server/main-channel-client.cpp @@ -684,8 +684,7 @@ static void main_channel_marshall_agent_data(RedChannelClient *rcc, { rcc->init_send_data(SPICE_MSG_MAIN_AGENT_DATA); /* since pipe item owns the data, keep it alive until it's sent */ - red_pipe_item_ref(&item->base); - spice_marshaller_add_by_ref_full(m, item->data, item->len, marshaller_unref_pipe_item, item); + item->base.add_to_marshaller(m, item->data, item->len); } static void main_channel_marshall_migrate_data_item(RedChannelClient *rcc, diff --git a/server/red-pipe-item.cpp b/server/red-pipe-item.cpp index 4581b94b..9d1ff8c4 100644 --- a/server/red-pipe-item.cpp +++ b/server/red-pipe-item.cpp @@ -47,8 +47,15 @@ void red_pipe_item_init_full(RedPipeItem *item, item->free_func = free_func ? free_func : (red_pipe_item_free_t *)g_free; } -void marshaller_unref_pipe_item(uint8_t *data G_GNUC_UNUSED, void *opaque) +static void marshaller_unref_pipe_item(uint8_t *, void *opaque) { RedPipeItem *item = (RedPipeItem*) opaque; red_pipe_item_unref(item); } + +void RedPipeItem::add_to_marshaller(SpiceMarshaller *m, uint8_t *data, size_t size) +{ + red_pipe_item_ref(this); + spice_marshaller_add_by_ref_full(m, data, size, + marshaller_unref_pipe_item, this); +} diff --git a/server/red-pipe-item.h b/server/red-pipe-item.h index 42a29926..5d81aacb 100644 --- a/server/red-pipe-item.h +++ b/server/red-pipe-item.h @@ -22,7 +22,9 @@ #include #include -SPICE_BEGIN_DECLS +#include "common/marshaller.h" + +#include "push-visibility.h" typedef struct RedPipeItem RedPipeItem; @@ -31,6 +33,8 @@ typedef void red_pipe_item_free_t(RedPipeItem *item); struct RedPipeItem { int type; + void add_to_marshaller(SpiceMarshaller *m, uint8_t *data, size_t size); + /* private */ int refcount; @@ -46,9 +50,6 @@ static inline void red_pipe_item_init(RedPipeItem *item, int type) red_pipe_item_init_full(item, type, NULL); } -/* a convenience function for unreffing a pipe item after it has been sent */ -void marshaller_unref_pipe_item(uint8_t *data, void *opaque); - -SPICE_END_DECLS +#include "pop-visibility.h" #endif /* RED_PIPE_ITEM_H_ */ diff --git a/server/smartcard-channel-client.cpp b/server/smartcard-channel-client.cpp index 6ee692e0..40d98989 100644 --- a/server/smartcard-channel-client.cpp +++ b/server/smartcard-channel-client.cpp @@ -121,12 +121,10 @@ void smartcard_channel_client_send_data(RedChannelClient *rcc, { spice_assert(rcc); spice_assert(vheader); + rcc->init_send_data(SPICE_MSG_SMARTCARD_DATA); /* NOTE: 'vheader' is assumed to be owned by 'item' so we keep the pipe * item valid until the message is actually sent. */ - red_pipe_item_ref(item); - rcc->init_send_data(SPICE_MSG_SMARTCARD_DATA); - spice_marshaller_add_by_ref_full(m, (uint8_t*)vheader, sizeof(VSCMsgHeader) + vheader->length, - marshaller_unref_pipe_item, item); + item->add_to_marshaller(m, (uint8_t*)vheader, sizeof(VSCMsgHeader) + vheader->length); } void smartcard_channel_client_send_error(RedChannelClient *rcc, SpiceMarshaller *m, RedPipeItem *item) diff --git a/server/spicevmc.cpp b/server/spicevmc.cpp index 8f1d8cfb..bb7a0216 100644 --- a/server/spicevmc.cpp +++ b/server/spicevmc.cpp @@ -532,9 +532,7 @@ static void spicevmc_red_channel_send_data(VmcChannelClient *rcc, }; spice_marshall_SpiceMsgCompressedData(m, &compressed_msg); } - red_pipe_item_ref(item); - spice_marshaller_add_by_ref_full(m, i->buf, i->buf_used, - marshaller_unref_pipe_item, item); + item->add_to_marshaller(m, i->buf, i->buf_used); // account for sent data and wake up device if was blocked uint32_t old_queued_data = channel->queued_data; diff --git a/server/stream-channel.cpp b/server/stream-channel.cpp index 81fb1ce7..bcccbe5d 100644 --- a/server/stream-channel.cpp +++ b/server/stream-channel.cpp @@ -234,9 +234,7 @@ void StreamChannelClient::send_item(RedPipeItem *pipe_item) StreamDataItem *item = SPICE_UPCAST(StreamDataItem, pipe_item); init_send_data(SPICE_MSG_DISPLAY_STREAM_DATA); spice_marshall_msg_display_stream_data(m, &item->data); - red_pipe_item_ref(pipe_item); - spice_marshaller_add_by_ref_full(m, item->data.data, item->data.data_size, - marshaller_unref_pipe_item, pipe_item); + pipe_item->add_to_marshaller(m, item->data.data, item->data.data_size); record(stream_channel_data, "Stream data packet size %u mm_time %u", item->data.data_size, item->data.base.multi_media_time); break;