[Spice-devel] [spice-server 06/10] qxl: Add red_cursor_cmd_new and red_cursor_cmd_free helpers

Christophe Fergeau cfergeau at redhat.com
Mon Apr 16 10:13:55 UTC 2018


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]
---
 server/cursor-channel.c         |  6 +-----
 server/red-parse-qxl.c          | 24 +++++++++++++++++++++---
 server/red-parse-qxl.h          |  6 +++---
 server/red-worker.c             | 10 +++++-----
 server/tests/test-qxl-parsing.c | 37 ++++++++++++++++++++-----------------
 5 files changed, 50 insertions(+), 33 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index ada1d62a7..c826627bf 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -69,13 +69,9 @@ static RedCursorPipeItem *cursor_pipe_item_new(RedCursorCmd *cmd)
 
 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_free(pipe_item->red_cursor);
     g_free(pipe_item);
 }
 
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index c08d18160..eb80ba81b 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1457,8 +1457,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, RedMemSlotInfo *slots,
+                               int group_id, RedCursorCmd *red,
+                               QXLPHYSICAL addr)
 {
     QXLCursorCmd *qxl_cmd;
     int error;
@@ -1467,6 +1468,7 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
     if (error) {
         return false;
     }
+    red->qxl = qxl;
     red->release_info_ext.info      = &qxl_cmd->release_info;
     red->release_info_ext.group_id  = group_id;
 
@@ -1484,10 +1486,25 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
         red->u.trail.frequency = qxl_cmd->u.trail.frequency;
         break;
     }
+
     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);
+    if (!red_get_cursor_cmd(qxl, slots, group_id, cmd, addr)) {
+        g_free(cmd);
+        return NULL;
+    }
+
+    return cmd;
+}
+
+void red_cursor_cmd_free(RedCursorCmd *red)
 {
     switch (red->type) {
     case QXL_CURSOR_SET:
@@ -1497,6 +1514,7 @@ void red_put_cursor_cmd(RedCursorCmd *red)
     if (red->qxl) {
         red_qxl_release_resource(red->qxl, red->release_info_ext);
     }
+    g_free(red);
 }
 
 void red_drawable_unref(RedDrawable *red_drawable)
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index 882657e88..231844e96 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -138,8 +138,8 @@ 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);
+void red_cursor_cmd_free(RedCursorCmd *cmd);
 
 #endif /* RED_PARSE_QXL_H_ */
diff --git a/server/red-worker.c b/server/red-worker.c
index ebc6d423d..2dcf2b0ef 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -103,13 +103,14 @@ 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);
+
     return TRUE;
 }
 
@@ -965,7 +966,6 @@ 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);
 
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 47139a48c..f2b12ad07 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_free(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_free(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_free(red_cursor_cmd);
     }
     g_test_assert_expected_messages();
 
-- 
2.14.3



More information about the Spice-devel mailing list