[Spice-devel] [PATCH spice-server v2 modified] Refactor cursor marshalling for SET, INIT

[Frediano Ziglio]

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 | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

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..905a85d 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -123,26 +123,34 @@ static RedPipeItem *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int
     return &item->base;
 }
 
-typedef struct {
-    void *data;
-    uint32_t size;
-} AddBufInfo;
+static void marshaller_free_cursor_item(uint8_t *data, void *opaque)
+{
+    CursorItem *item = opaque;
+    cursor_item_unref(item);
+}
 
-static void add_buf_from_info(SpiceMarshaller *m, AddBufInfo *info)
+static void marshall_cursor_data(SpiceMarshaller *m, CursorItem *cursor_item,
+                                 const SpiceCursor *red_cursor)
 {
-    if (info->data) {
-        spice_marshaller_add_by_ref(m, info->data, info->size);
+    spice_assert(red_cursor != NULL);
+    /* the cursor data is owned by 'cursor_item', so we need to ensure that 'cursor_item'
+     * remains valid until the data is sent. */
+    if (red_cursor->data != NULL && red_cursor->data_size && cursor_item != NULL) {
+        cursor_item_ref(cursor_item);
+        spice_marshaller_add_by_ref_full(m, red_cursor->data, red_cursor->data_size,
+                                         marshaller_free_cursor_item, cursor_item);
     }
 }
 
 static void cursor_fill(CursorChannelClient *ccc, CursorItem *cursor,
-                        SpiceCursor *red_cursor, AddBufInfo *addbuf)
+                        SpiceCursor *red_cursor)
 {
     RedCursorCmd *cursor_cmd;
-    addbuf->data = NULL;
 
     if (!cursor) {
         red_cursor->flags = SPICE_CURSOR_FLAGS_NONE;
+        red_cursor->data_size = 0;
+        red_cursor->data = NULL;
         return;
     }
 
@@ -158,11 +166,6 @@ static void cursor_fill(CursorChannelClient *ccc, CursorItem *cursor,
             red_cursor->flags |= SPICE_CURSOR_FLAGS_CACHE_ME;
         }
     }
-
-    if (red_cursor->data_size) {
-        addbuf->data = red_cursor->data;
-        addbuf->size = red_cursor->data_size;
-    }
 }
 
 void cursor_channel_disconnect(CursorChannel *cursor_channel)
@@ -192,7 +195,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 +205,9 @@ 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);
     spice_marshall_msg_cursor_init(base_marshaller, &msg);
-    add_buf_from_info(base_marshaller, &info);
+    marshall_cursor_data(base_marshaller, cursor_channel->item, &msg.cursor);
 }
 
 static void cursor_marshall(CursorChannelClient *ccc,
@@ -233,15 +235,14 @@ 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);
             spice_marshall_msg_cursor_set(m, &cursor_set);
-            add_buf_from_info(m, &info);
+            marshall_cursor_data(m, item, &cursor_set.cursor);
             break;
         }
     case QXL_CURSOR_HIDE:
-- 
2.9.3