I like it. I'll include this in the new patch series.
On Sun, 2016-12-18 at 13:11 +0000, 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 | 31 +++++++++++--------------------
> 1 file changed, 11 insertions(+), 20 deletions(-)
>
> Changes sinvce v2 modified:
> - use submarshaller. This make code much simpler avoiding
> to define extra structure and an additional function.
>
> Changes sinvce v2:
> - avoid to use AddBufInfo structure, it's confusing and
> all fields are in CursorItem/SpiceCursor already;
> This make cursor_fill change just a parameter, 2 is
> confusing.
>
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index fded226..837d3a8 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -123,23 +123,16 @@ static RedPipeItem
> *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int
> return &item->base;
> }
>
> -typedef struct {
> - void *data;
> - uint32_t size;
> -} AddBufInfo;
> -
> -static void add_buf_from_info(SpiceMarshaller *m, AddBufInfo *info)
> +static void marshaller_free_cursor_item(uint8_t *data, void *opaque)
> {
> - if (info->data) {
> - spice_marshaller_add_by_ref(m, info->data, info->size);
> - }
> + CursorItem *item = opaque;
> + cursor_item_unref(item);
> }
>
> static void cursor_fill(CursorChannelClient *ccc, CursorItem
> *cursor,
> - SpiceCursor *red_cursor, AddBufInfo *addbuf)
> + SpiceCursor *red_cursor, SpiceMarshaller *m)
> {
> RedCursorCmd *cursor_cmd;
> - addbuf->data = NULL;
>
> if (!cursor) {
> red_cursor->flags = SPICE_CURSOR_FLAGS_NONE;
> @@ -160,8 +153,10 @@ static void cursor_fill(CursorChannelClient
> *ccc, CursorItem *cursor,
> }
>
> if (red_cursor->data_size) {
> - addbuf->data = red_cursor->data;
> - addbuf->size = red_cursor->data_size;
> + SpiceMarshaller *m2 = spice_marshaller_get_submarshaller(m);
> + cursor_item_ref(cursor);
> + spice_marshaller_add_by_ref_full(m2, red_cursor->data,
> red_cursor->data_size,
> + marshaller_free_cursor_item
> , cursor);
> }
> }
>
> @@ -192,7 +187,6 @@ static void
> red_marshall_cursor_init(CursorChannelClient *ccc, SpiceMarshaller *
> CursorChannel *cursor_channel;
> RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> SpiceMsgCursorInit msg;
> - AddBufInfo info;
>
> spice_assert(rcc);
> cursor_channel =
> CURSOR_CHANNEL(red_channel_client_get_channel(rcc));
> @@ -203,9 +197,8 @@ static void
> red_marshall_cursor_init(CursorChannelClient *ccc, SpiceMarshaller *
> msg.trail_length = cursor_channel->cursor_trail_length;
> msg.trail_frequency = cursor_channel->cursor_trail_frequency;
>
> - cursor_fill(ccc, cursor_channel->item, &msg.cursor, &info);
> + cursor_fill(ccc, cursor_channel->item, &msg.cursor,
> base_marshaller);
> spice_marshall_msg_cursor_init(base_marshaller, &msg);
> - add_buf_from_info(base_marshaller, &info);
> }
>
> static void cursor_marshall(CursorChannelClient *ccc,
> @@ -233,15 +226,13 @@ static void cursor_marshall(CursorChannelClient
> *ccc,
> case QXL_CURSOR_SET:
> {
> 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);
> + cursor_fill(ccc, item, &cursor_set.cursor, m);
> spice_marshall_msg_cursor_set(m, &cursor_set);
> - add_buf_from_info(m, &info);
> break;
> }
> case QXL_CURSOR_HIDE:
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel