On Fri, 2016-12-16 at 11:23 -0500, Frediano Ziglio wrote: > > > > Use spice_marshaller_add_by_ref_full() instead of > > spice_marshaller_add_by_ref() to allow the marshaller to manage the > > lifetime of the referenced data buffer rather than having to manage > > it > > by passing a PipeItem to red_channel_client_init_send_data(). Since > > the > > data is owned by CursorItem (which is not in fact a RedPipeItem, > > but is > > owned by a pipe item and is itself refcounted), we take a reference > > on > > the CursorItem when adding the data buf to the marshaller, and then > > unreference it in the marshaller free func. > > --- > > server/cursor-channel.c | 27 ++++++++++++++++++++------- > > 1 file changed, 20 insertions(+), 7 deletions(-) > > > > diff --git a/server/cursor-channel.c b/server/cursor-channel.c > > index 04483af..8befa6e 100644 > > --- a/server/cursor-channel.c > > +++ b/server/cursor-channel.c > > @@ -126,13 +126,26 @@ static RedPipeItem > > *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int > > typedef struct { > > void *data; > > uint32_t size; > > + CursorItem *item; > > } AddBufInfo; > > > > This field here does not look right... I'd honestly remove > all the AddBufInfo structure, I'll post a possible different > patch. I didn't like it either. In fact, I tried 2 different approaches before this, but wasn't happy with either of them. So I decided to just go this route. If you can come up with a more elegant solution, I'm happy to use your patch. > > > > > -static void add_buf_from_info(SpiceMarshaller *m, AddBufInfo > > *info) > > +static void marshaller_free_cursor_item(uint8_t *data, void > > *opaque) > > +{ > > + CursorItem *item = opaque; > > + if (item) > > + cursor_item_unref(item); > > +} > > + > > +static void marshall_cursor_data(SpiceMarshaller *m, AddBufInfo > > *info) > > { > > - if (info->data) { > > - spice_marshaller_add_by_ref(m, info->data, info->size); > > + spice_return_if_fail(info != NULL); > > + /* the cursor data is owned by 'item', so we need to ensure > > that 'item' > > + * remains valid until the data is sent. */ > > + if (info->item) { > > + cursor_item_ref(info->item); > > } > > + spice_marshaller_add_by_ref_full(m, info->data, info->size, > > + marshaller_free_cursor_item, > > info->item); > > } > > > > static void cursor_fill(CursorChannelClient *ccc, CursorItem > > *cursor, > > @@ -140,6 +153,7 @@ static void cursor_fill(CursorChannelClient > > *ccc, > > CursorItem *cursor, > > { > > RedCursorCmd *cursor_cmd; > > addbuf->data = NULL; > > + addbuf->item = cursor; > > > > if (!cursor) { > > red_cursor->flags = SPICE_CURSOR_FLAGS_NONE; > > @@ -205,7 +219,7 @@ static void > > red_marshall_cursor_init(CursorChannelClient > > *ccc, SpiceMarshaller * > > > > cursor_fill(ccc, cursor_channel->item, &msg.cursor, &info); > > spice_marshall_msg_cursor_init(base_marshaller, &msg); > > - add_buf_from_info(base_marshaller, &info); > > + marshall_cursor_data(base_marshaller, &info); > > } > > > > static void cursor_marshall(CursorChannelClient *ccc, > > @@ -215,7 +229,6 @@ static void cursor_marshall(CursorChannelClient > > *ccc, > > RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc); > > CursorChannel *cursor_channel = > > CURSOR_CHANNEL(red_channel_client_get_channel(rcc)); > > CursorItem *item = cursor_pipe_item->cursor_item; > > - RedPipeItem *pipe_item = &cursor_pipe_item->base; > > RedCursorCmd *cmd; > > > > spice_return_if_fail(cursor_channel); > > @@ -235,13 +248,13 @@ static void > > cursor_marshall(CursorChannelClient *ccc, > > SpiceMsgCursorSet cursor_set; > > AddBufInfo info; > > > > - red_channel_client_init_send_data(rcc, > > SPICE_MSG_CURSOR_SET, > > pipe_item); > > + red_channel_client_init_send_data(rcc, > > SPICE_MSG_CURSOR_SET, > > NULL); > > cursor_set.position = cmd->u.set.position; > > cursor_set.visible = cursor_channel->cursor_visible; > > > > cursor_fill(ccc, item, &cursor_set.cursor, &info); > > spice_marshall_msg_cursor_set(m, &cursor_set); > > - add_buf_from_info(m, &info); > > + marshall_cursor_data(m, &info); > > break; > > } > > case QXL_CURSOR_HIDE: > > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel