red-pipe-item: Better encapsulate marshaller_unref_pipe_item

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 <freddy77@gmail.com>
Acked-by: Julien Ropé <jrope@gmail.com>
This commit is contained in:
Frediano Ziglio 2020-06-05 14:03:35 +01:00
parent 65d7d24f21
commit 45e964dc5a
8 changed files with 22 additions and 25 deletions

View File

@ -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);
}
}

View File

@ -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, &copy.base.box);
}
spice_chunks_destroy(chunks);

View File

@ -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,

View File

@ -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);
}

View File

@ -22,7 +22,9 @@
#include <stddef.h>
#include <inttypes.h>
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_ */

View File

@ -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)

View File

@ -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;

View File

@ -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;