[Spice-devel] [PATCH spice-server v3 6/6] cursor-channel: Use a single RedCursorPipeItem to hold the cursor

Frediano Ziglio fziglio at redhat.com
Mon Sep 4 15:02:58 UTC 2017


RedPipeItem already implements reference counting so
this avoid duplicating code to handle a object with reference
counting that points to another object with reference counting
that holds a RedCursorCmd.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Christophe Fergeau <cfergeau at redhat.com>
---
 server/cursor-channel.c | 114 +++++++++++++++---------------------------------
 1 file changed, 35 insertions(+), 79 deletions(-)

Changes since v2:
- remove cursor_pipe_item_ref.

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index 2441e1ed..8746b9a6 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -28,17 +28,17 @@
 #include "reds.h"
 #include "red-qxl.h"
 
-typedef struct CursorItem {
+typedef struct RedCursorPipeItem {
+    RedPipeItem base;
     QXLInstance *qxl;
-    int refs;
     RedCursorCmd *red_cursor;
-} CursorItem;
+} RedCursorPipeItem;
 
 struct CursorChannel
 {
     CommonGraphicsChannel parent;
 
-    CursorItem *item;
+    RedCursorPipeItem *item;
     bool cursor_visible;
     SpicePoint16 cursor_position;
     uint16_t cursor_trail_length;
@@ -51,83 +51,49 @@ struct CursorChannelClass
     CommonGraphicsChannelClass parent_class;
 };
 
-typedef struct RedCursorPipeItem {
-    RedPipeItem base;
-    CursorItem *cursor_item;
-} RedCursorPipeItem;
-
 G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
 
 static void cursor_pipe_item_free(RedPipeItem *pipe_item);
 
-static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
+static RedCursorPipeItem *cursor_pipe_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
 {
-    CursorItem *cursor_item;
+    RedCursorPipeItem *item = spice_malloc0(sizeof(RedCursorPipeItem));
 
     spice_return_val_if_fail(cmd != NULL, NULL);
 
-    cursor_item = g_new0(CursorItem, 1);
-    cursor_item->qxl = qxl;
-    cursor_item->refs = 1;
-    cursor_item->red_cursor = cmd;
-
-    return cursor_item;
-}
-
-static CursorItem *cursor_item_ref(CursorItem *item)
-{
-    spice_return_val_if_fail(item != NULL, NULL);
-    spice_return_val_if_fail(item->refs > 0, NULL);
-
-    item->refs++;
+    red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
+                            cursor_pipe_item_free);
+    item->qxl = qxl;
+    item->red_cursor = cmd;
 
     return item;
 }
 
-static void cursor_item_unref(CursorItem *item)
+static void cursor_pipe_item_free(RedPipeItem *base)
 {
     RedCursorCmd *cursor_cmd;
+    RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
 
-    spice_return_if_fail(item != NULL);
-
-    if (--item->refs)
-        return;
-
-    cursor_cmd = item->red_cursor;
-    red_qxl_release_resource(item->qxl, cursor_cmd->release_info_ext);
+    cursor_cmd = pipe_item->red_cursor;
+    red_qxl_release_resource(pipe_item->qxl, cursor_cmd->release_info_ext);
     red_put_cursor_cmd(cursor_cmd);
     free(cursor_cmd);
 
-    g_free(item);
-
-}
-
-static void cursor_channel_set_item(CursorChannel *cursor, CursorItem *item)
-{
-    if (cursor->item)
-        cursor_item_unref(cursor->item);
-
-    cursor->item = item ? cursor_item_ref(item) : NULL;
-}
-
-static RedPipeItem *new_cursor_pipe_item(CursorItem *cursor_item)
-{
-    RedCursorPipeItem *item = spice_malloc0(sizeof(RedCursorPipeItem));
-
-    red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
-                            cursor_pipe_item_free);
-    item->cursor_item = cursor_item;
-    item->cursor_item->refs++;
-    return &item->base;
+    free(pipe_item);
 }
 
-static void marshaller_unref_cursor_item(uint8_t *data, void *opaque)
+static void cursor_channel_set_item(CursorChannel *cursor, RedCursorPipeItem *item)
 {
-    CursorItem *item = opaque;
-    cursor_item_unref(item);
+    if (item) {
+        red_pipe_item_ref(&item->base);
+    }
+    if (cursor->item) {
+        red_pipe_item_unref(&cursor->item->base);
+    }
+    cursor->item = item;
 }
 
-static void cursor_fill(CursorChannelClient *ccc, CursorItem *cursor,
+static void cursor_fill(CursorChannelClient *ccc, RedCursorPipeItem *cursor,
                         SpiceCursor *red_cursor, SpiceMarshaller *m)
 {
     RedCursorCmd *cursor_cmd;
@@ -152,22 +118,12 @@ static void cursor_fill(CursorChannelClient *ccc, CursorItem *cursor,
 
     if (red_cursor->data_size) {
         SpiceMarshaller *m2 = spice_marshaller_get_submarshaller(m);
-        cursor_item_ref(cursor);
+        red_pipe_item_ref(&cursor->base);
         spice_marshaller_add_by_ref_full(m2, red_cursor->data, red_cursor->data_size,
-                                         marshaller_unref_cursor_item, cursor);
+                                         marshaller_unref_pipe_item, &cursor->base);
     }
 }
 
-static void cursor_pipe_item_free(RedPipeItem *base)
-{
-    spice_return_if_fail(base);
-
-    RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
-
-    cursor_item_unref(pipe_item->cursor_item);
-    free(pipe_item);
-}
-
 static void red_marshall_cursor_init(CursorChannelClient *ccc, SpiceMarshaller *base_marshaller,
                                      RedPipeItem *pipe_item)
 {
@@ -195,7 +151,7 @@ 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;
+    RedCursorPipeItem *item = cursor_pipe_item;
     RedCursorCmd *cmd;
 
     spice_return_if_fail(cursor_channel);
@@ -296,7 +252,7 @@ CursorChannel* cursor_channel_new(RedsState *server, QXLInstance *qxl,
 
 void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
 {
-    CursorItem *cursor_item;
+    RedCursorPipeItem *cursor_pipe_item;
     bool cursor_show = false;
     QXLInstance *qxl;
 
@@ -304,12 +260,12 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
     spice_return_if_fail(cursor_cmd);
 
     qxl = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(cursor));
-    cursor_item = cursor_item_new(qxl, cursor_cmd);
+    cursor_pipe_item = cursor_pipe_item_new(qxl, cursor_cmd);
 
     switch (cursor_cmd->type) {
     case QXL_CURSOR_SET:
         cursor->cursor_visible = !!cursor_cmd->u.set.visible;
-        cursor_channel_set_item(cursor, cursor_item);
+        cursor_channel_set_item(cursor, cursor_pipe_item);
         break;
     case QXL_CURSOR_MOVE:
         cursor_show = !cursor->cursor_visible;
@@ -325,7 +281,7 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
         break;
     default:
         spice_warning("invalid cursor command %u", cursor_cmd->type);
-        cursor_item_unref(cursor_item);
+        red_pipe_item_unref(&cursor_pipe_item->base);
         return;
     }
 
@@ -333,10 +289,10 @@ void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd *cursor_cmd)
         (cursor->mouse_mode == SPICE_MOUSE_MODE_SERVER
          || cursor_cmd->type != QXL_CURSOR_MOVE
          || cursor_show)) {
-        red_channel_pipes_add(RED_CHANNEL(cursor), new_cursor_pipe_item(cursor_item));
+        red_channel_pipes_add(RED_CHANNEL(cursor), &cursor_pipe_item->base);
+    } else {
+        red_pipe_item_unref(&cursor_pipe_item->base);
     }
-
-    cursor_item_unref(cursor_item);
 }
 
 void cursor_channel_reset(CursorChannel *cursor)
@@ -419,7 +375,7 @@ cursor_channel_finalize(GObject *object)
     CursorChannel *self = CURSOR_CHANNEL(object);
 
     if (self->item) {
-        cursor_item_unref(self->item);
+        red_pipe_item_unref(&self->item->base);
     }
 
     G_OBJECT_CLASS(cursor_channel_parent_class)->finalize(object);
-- 
2.13.5



More information about the Spice-devel mailing list