qxl: Add red_cursor_cmd_{new,ref,unref} helpers

Currently, the cursor channel is allocating RedCursorCmd instances itself, and then
calling into red-parse-qxl.h to initialize it, and doing something
similar when releasing the data. This commit moves this common code to
red-parse-qxl.[ch]
The ref/unref are not strictly needed, red_cursor_cmd_free() would
currently be enough, but this makes the API consistent with
red_drawable_{new,ref,unref}.

5 files changed, 70 insertions, 35 deletions

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index ada1d62a..bb85f0ed 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -62,20 +62,16 @@ static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd)
 
     red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
                             cursor_pipe_item_free);
-    item->red_cursor = cmd;
+    item->red_cursor = red_cursor_cmd_ref(cmd);
 
     return item;
 }
 
 static void cursor_pipe_item_free(RedPipeItem *base)
 {
-    RedCursorCmd *cursor_cmd;
     RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
 
-    cursor_cmd = pipe_item->red_cursor;
-    red_put_cursor_cmd(cursor_cmd);
-    g_free(cursor_cmd);
-
+    red_cursor_cmd_unref(pipe_item->red_cursor);
     g_free(pipe_item);
 }
 
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index b819df63..b176e55c 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1444,8 +1444,9 @@ static void red_put_cursor(SpiceCursor *red)
     g_free(red->data);
 }
 
-bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                        RedCursorCmd *red, QXLPHYSICAL addr)
+static bool red_get_cursor_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+                               int group_id, RedCursorCmd *red,
+                               QXLPHYSICAL addr)
 {
     QXLCursorCmd *qxl;
     int error;
@@ -1454,6 +1455,7 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     if (error) {
         return false;
     }
+    red->qxl = qxl_instance;
     red->release_info_ext.info      = &qxl->release_info;
     red->release_info_ext.group_id  = group_id;
 
@@ -1474,7 +1476,24 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     return true;
 }
 
-void red_put_cursor_cmd(RedCursorCmd *red)
+RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr)
+{
+    RedCursorCmd *cmd;
+
+    cmd = g_new0(RedCursorCmd, 1);
+
+    cmd->refs = 1;
+
+    if (!red_get_cursor_cmd(qxl, slots, group_id, cmd, addr)) {
+        g_free(cmd);
+        return NULL;
+    }
+
+    return cmd;
+}
+
+static void red_put_cursor_cmd(RedCursorCmd *red)
 {
     switch (red->type) {
     case QXL_CURSOR_SET:
@@ -1486,6 +1505,21 @@ void red_put_cursor_cmd(RedCursorCmd *red)
     }
 }
 
+RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *red)
+{
+    red->refs++;
+    return red;
+}
+
+void red_cursor_cmd_unref(RedCursorCmd *red)
+{
+    if (--red->refs) {
+        return;
+    }
+    red_put_cursor_cmd(red);
+    g_free(red);
+}
+
 void red_drawable_unref(RedDrawable *red_drawable)
 {
     if (--red_drawable->refs) {
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 0e67aa8f..30b77921 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -104,6 +104,7 @@ typedef struct RedSurfaceCmd {
 typedef struct RedCursorCmd {
     QXLInstance *qxl;
     QXLReleaseInfoExt release_info_ext;
+    int refs;
     uint8_t type;
     union {
         struct {
@@ -136,8 +137,9 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
                          RedSurfaceCmd *red, QXLPHYSICAL addr);
 void red_put_surface_cmd(RedSurfaceCmd *red);
 
-bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
-                        RedCursorCmd *red, QXLPHYSICAL addr);
-void red_put_cursor_cmd(RedCursorCmd *red);
+RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
+                                 int group_id, QXLPHYSICAL addr);
+RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *cmd);
+void red_cursor_cmd_unref(RedCursorCmd *cmd);
 
 #endif /* RED_PARSE_QXL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index ebc6d423..83e699ed 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -103,13 +103,15 @@ static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *e
 {
     RedCursorCmd *cursor_cmd;
 
-    cursor_cmd = g_new0(RedCursorCmd, 1);
-    if (!red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
-        g_free(cursor_cmd);
+    cursor_cmd = red_cursor_cmd_new(worker->qxl, &worker->mem_slots,
+                                    ext->group_id, ext->cmd.data);
+    if (cursor_cmd == NULL) {
         return FALSE;
     }
-    cursor_cmd->qxl = worker->qxl;
+
     cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
+    red_cursor_cmd_unref(cursor_cmd);
+
     return TRUE;
 }
 
@@ -965,10 +967,8 @@ static bool loadvm_command(RedWorker *worker, QXLCommandExt *ext)
     switch (ext->cmd.type) {
     case QXL_CMD_CURSOR:
         return red_process_cursor_cmd(worker, ext);
-
     case QXL_CMD_SURFACE:
         return red_process_surface_cmd(worker, ext, TRUE);
-
     default:
         spice_warning("unhandled loadvm command type (%d)", ext->cmd.type);
     }
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 47139a48..e757bdfa 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -149,7 +149,7 @@ static void test_too_big_image(void)
 static void test_cursor_command(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
 
@@ -167,8 +167,9 @@ static void test_cursor_command(void)
 
     cursor_cmd.u.set.shape = to_physical(cursor);
 
-    g_assert_true(red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd)));
-    g_free(red_cursor_cmd.u.set.shape.data);
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    g_assert_true(red_cursor_cmd != NULL);
+    red_cursor_cmd_unref(red_cursor_cmd);
     g_free(cursor);
     memslot_info_destroy(&mem_info);
 }
@@ -176,7 +177,7 @@ static void test_cursor_command(void)
 static void test_circular_empty_chunks(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
     QXLDataChunk *chunks[2];
@@ -200,13 +201,14 @@ static void test_circular_empty_chunks(void)
 
     cursor_cmd.u.set.shape = to_physical(cursor);
 
-    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
-    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    if (red_cursor_cmd != NULL) {
         /* function does not return errors so there should be no data */
-        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
+        red_cursor_cmd_unref(red_cursor_cmd);
     }
     g_test_assert_expected_messages();
 
@@ -218,7 +220,7 @@ static void test_circular_empty_chunks(void)
 static void test_circular_small_chunks(void)
 {
     RedMemSlotInfo mem_info;
-    RedCursorCmd red_cursor_cmd;
+    RedCursorCmd *red_cursor_cmd;
     QXLCursorCmd cursor_cmd;
     QXLCursor *cursor;
     QXLDataChunk *chunks[2];
@@ -242,13 +244,14 @@ static void test_circular_small_chunks(void)
 
     cursor_cmd.u.set.shape = to_physical(cursor);
 
-    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
-    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd));
+    if (red_cursor_cmd != NULL) {
         /* function does not return errors so there should be no data */
-        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
-        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
+        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
+        red_cursor_cmd_unref(red_cursor_cmd);
     }
     g_test_assert_expected_messages();