[Spice-commits] 7 commits - server/dcc.c server/dcc.h server/dcc-send.c server/display-channel.c server/Makefile.am server/red-channel.c server/red-channel.h server/red-pipe-item.c server/red-pipe-item.h server/reds.c server/smartcard.c server/spicevmc.c server/stream.c server/stream.h

Jonathon Jongsma jjongsma at kemper.freedesktop.org
Fri Apr 15 15:32:35 UTC 2016


 server/Makefile.am       |    2 +
 server/dcc-send.c        |    4 +-
 server/dcc.c             |   51 +++++++++-----------------------
 server/dcc.h             |    6 ---
 server/display-channel.c |    8 +----
 server/red-channel.c     |    6 ---
 server/red-channel.h     |   13 --------
 server/red-pipe-item.c   |   53 +++++++++++++++++++++++++++++++++
 server/red-pipe-item.h   |   46 +++++++++++++++++++++++++++++
 server/reds.c            |   74 ++++++++++++++++++++++++-----------------------
 server/smartcard.c       |   35 +++++++---------------
 server/spicevmc.c        |   25 +++------------
 server/stream.c          |   30 +++++++++----------
 server/stream.h          |    6 ---
 14 files changed, 193 insertions(+), 166 deletions(-)

New commits:
commit e09598ec75ff86c533050e9671cee836c2311c62
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Thu Apr 14 15:32:20 2016 -0500

    StreamClipItem: use base class refcounting
    
    PipeItem already implements refcounting. Use it.

diff --git a/server/dcc.c b/server/dcc.c
index 234246f..daf3180 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -497,7 +497,7 @@ void dcc_stop(DisplayChannelClient *dcc)
 
 void dcc_stream_agent_clip(DisplayChannelClient* dcc, StreamAgent *agent)
 {
-    StreamClipItem *item = stream_clip_item_new(dcc, agent);
+    StreamClipItem *item = stream_clip_item_new(agent);
     int n_rects;
 
     item->clip_type = SPICE_CLIP_TYPE_RECTS;
@@ -1601,10 +1601,8 @@ static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item)
     switch (item->type) {
     case PIPE_ITEM_TYPE_DRAW:
     case PIPE_ITEM_TYPE_IMAGE:
-        pipe_item_unref(item);
-        break;
     case PIPE_ITEM_TYPE_STREAM_CLIP:
-        stream_clip_item_unref(dcc, (StreamClipItem *)item);
+        pipe_item_unref(item);
         break;
     case PIPE_ITEM_TYPE_UPGRADE:
         upgrade_item_unref(display, (UpgradeItem *)item);
@@ -1645,14 +1643,12 @@ static void release_item_before_push(DisplayChannelClient *dcc, PipeItem *item)
         stream_agent_unref(display, agent);
         break;
     }
-    case PIPE_ITEM_TYPE_STREAM_CLIP:
-        stream_clip_item_unref(dcc, (StreamClipItem *)item);
-        break;
     case PIPE_ITEM_TYPE_STREAM_DESTROY: {
         StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item);
         stream_agent_unref(display, agent);
         break;
     }
+    case PIPE_ITEM_TYPE_STREAM_CLIP:
     case PIPE_ITEM_TYPE_UPGRADE:
         upgrade_item_unref(display, (UpgradeItem *)item);
         break;
diff --git a/server/display-channel.c b/server/display-channel.c
index 4d339f5..88dbc74 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1972,10 +1972,8 @@ static void hold_item(RedChannelClient *rcc, PipeItem *item)
     switch (item->type) {
     case PIPE_ITEM_TYPE_DRAW:
     case PIPE_ITEM_TYPE_IMAGE:
-        pipe_item_ref(item);
-        break;
     case PIPE_ITEM_TYPE_STREAM_CLIP:
-        ((StreamClipItem *)item)->refs++;
+        pipe_item_ref(item);
         break;
     case PIPE_ITEM_TYPE_UPGRADE:
         ((UpgradeItem *)item)->refs++;
diff --git a/server/stream.c b/server/stream.c
index 548c4c7..ae37a62 100644
--- a/server/stream.c
+++ b/server/stream.c
@@ -133,29 +133,29 @@ void stream_agent_unref(DisplayChannel *display, StreamAgent *agent)
     stream_unref(display, agent->stream);
 }
 
-StreamClipItem *stream_clip_item_new(DisplayChannelClient* dcc, StreamAgent *agent)
+void stream_clip_item_free(StreamClipItem *item)
 {
-    StreamClipItem *item = spice_new(StreamClipItem, 1);
-    pipe_item_init((PipeItem *)item, PIPE_ITEM_TYPE_STREAM_CLIP);
-
-    item->stream_agent = agent;
-    agent->stream->refs++;
-    item->refs = 1;
-    return item;
-}
-
-void stream_clip_item_unref(DisplayChannelClient *dcc, StreamClipItem *item)
-{
-    DisplayChannel *display = DCC_TO_DC(dcc);
+    g_return_if_fail(item != NULL);
+    DisplayChannel *display = DCC_TO_DC(item->stream_agent->dcc);
 
-    if (--item->refs != 0)
-        return;
+    g_return_if_fail(item->base.refcount == 0);
 
     stream_agent_unref(display, item->stream_agent);
     free(item->rects);
     free(item);
 }
 
+StreamClipItem *stream_clip_item_new(StreamAgent *agent)
+{
+    StreamClipItem *item = spice_new(StreamClipItem, 1);
+    pipe_item_init_full((PipeItem *)item, PIPE_ITEM_TYPE_STREAM_CLIP,
+                        (GDestroyNotify)stream_clip_item_free);
+
+    item->stream_agent = agent;
+    agent->stream->refs++;
+    return item;
+}
+
 static int is_stream_start(Drawable *drawable)
 {
     return ((drawable->frames_count >= RED_STREAM_FRAMES_START_CONDITION) &&
diff --git a/server/stream.h b/server/stream.h
index a3e84ed..c83e3b5 100644
--- a/server/stream.h
+++ b/server/stream.h
@@ -101,16 +101,12 @@ typedef struct StreamAgent {
 
 typedef struct StreamClipItem {
     PipeItem base;
-    int refs;
     StreamAgent *stream_agent;
     int clip_type;
     SpiceClipRects *rects;
 } StreamClipItem;
 
-StreamClipItem *      stream_clip_item_new                          (DisplayChannelClient* dcc,
-                                                                     StreamAgent *agent);
-void                  stream_clip_item_unref                        (DisplayChannelClient *dcc,
-                                                                     StreamClipItem *item);
+StreamClipItem *      stream_clip_item_new                          (StreamAgent *agent);
 
 typedef struct ItemTrace {
     red_time_t time;
commit 889a0b880f359cefc1fe07ed9256a9463fc7a749
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Thu Apr 14 15:22:59 2016 -0500

    DrawablePipeItem: use base class for refcounting
    
    Since PipeItem already implements refcounting, there's no need to
    re-implement it here.

diff --git a/server/dcc.c b/server/dcc.c
index 4e450b5..234246f 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -274,20 +274,14 @@ static void add_drawable_surface_images(DisplayChannelClient *dcc, Drawable *dra
     dcc_push_surface_image(dcc, drawable->surface_id);
 }
 
-DrawablePipeItem *drawable_pipe_item_ref(DrawablePipeItem *dpi)
-{
-    dpi->refs++;
-    return dpi;
-}
-
-void drawable_pipe_item_unref(DrawablePipeItem *dpi)
+void drawable_pipe_item_free(PipeItem *item)
 {
+    DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
     DisplayChannel *display = DCC_TO_DC(dpi->dcc);
 
-    if (--dpi->refs != 0)
-        return;
+    spice_assert(item->refcount == 0);
 
-    spice_warn_if_fail(!ring_item_is_linked(&dpi->dpi_pipe_item.link));
+    spice_warn_if_fail(!ring_item_is_linked(&item->link));
     spice_warn_if_fail(!ring_item_is_linked(&dpi->base));
     display_channel_drawable_unref(display, dpi->drawable);
     free(dpi);
@@ -302,8 +296,8 @@ static DrawablePipeItem *drawable_pipe_item_new(DisplayChannelClient *dcc, Drawa
     dpi->dcc = dcc;
     ring_item_init(&dpi->base);
     ring_add(&drawable->pipes, &dpi->base);
-    pipe_item_init(&dpi->dpi_pipe_item, PIPE_ITEM_TYPE_DRAW);
-    dpi->refs++;
+    pipe_item_init_full(&dpi->dpi_pipe_item, PIPE_ITEM_TYPE_DRAW,
+                        (GDestroyNotify)drawable_pipe_item_free);
     drawable->refs++;
     return dpi;
 }
@@ -1606,7 +1600,8 @@ static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item)
 
     switch (item->type) {
     case PIPE_ITEM_TYPE_DRAW:
-        drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
+    case PIPE_ITEM_TYPE_IMAGE:
+        pipe_item_unref(item);
         break;
     case PIPE_ITEM_TYPE_STREAM_CLIP:
         stream_clip_item_unref(dcc, (StreamClipItem *)item);
@@ -1614,9 +1609,6 @@ static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item)
     case PIPE_ITEM_TYPE_UPGRADE:
         upgrade_item_unref(display, (UpgradeItem *)item);
         break;
-    case PIPE_ITEM_TYPE_IMAGE:
-        pipe_item_unref(item);
-        break;
     case PIPE_ITEM_TYPE_GL_SCANOUT:
     case PIPE_ITEM_TYPE_GL_DRAW:
     case PIPE_ITEM_TYPE_VERB:
@@ -1645,7 +1637,7 @@ static void release_item_before_push(DisplayChannelClient *dcc, PipeItem *item)
     case PIPE_ITEM_TYPE_DRAW: {
         DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item);
         ring_remove(&dpi->base);
-        drawable_pipe_item_unref(dpi);
+        pipe_item_unref(item);
         break;
     }
     case PIPE_ITEM_TYPE_STREAM_CREATE: {
diff --git a/server/dcc.h b/server/dcc.h
index 3d7870b..e8354e2 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -157,9 +157,6 @@ typedef struct DrawablePipeItem {
     uint8_t refs;
 } DrawablePipeItem;
 
-void                       drawable_pipe_item_unref                  (DrawablePipeItem *dpi);
-DrawablePipeItem*          drawable_pipe_item_ref                    (DrawablePipeItem *dpi);
-
 DisplayChannelClient*      dcc_new                                   (DisplayChannel *display,
                                                                       RedClient *client,
                                                                       RedsStream *stream,
diff --git a/server/display-channel.c b/server/display-channel.c
index ac10724..4d339f5 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1971,7 +1971,8 @@ static void hold_item(RedChannelClient *rcc, PipeItem *item)
 
     switch (item->type) {
     case PIPE_ITEM_TYPE_DRAW:
-        drawable_pipe_item_ref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item));
+    case PIPE_ITEM_TYPE_IMAGE:
+        pipe_item_ref(item);
         break;
     case PIPE_ITEM_TYPE_STREAM_CLIP:
         ((StreamClipItem *)item)->refs++;
@@ -1979,9 +1980,6 @@ static void hold_item(RedChannelClient *rcc, PipeItem *item)
     case PIPE_ITEM_TYPE_UPGRADE:
         ((UpgradeItem *)item)->refs++;
         break;
-    case PIPE_ITEM_TYPE_IMAGE:
-        pipe_item_ref(item);
-        break;
     default:
         spice_warn_if_reached();
     }
commit 3bd6b215d382c62a919ef5ab9c32cc9cc32f65e4
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Thu Apr 14 14:56:03 2016 -0500

    dcc: use PipeItem refcounting for ImageItem
    
    Since the base class now implements refcounting, there's no need to
    re-invent it here.

diff --git a/server/dcc-send.c b/server/dcc-send.c
index eb866cf..b8d1157 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -728,7 +728,7 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
 
         spice_assert(image);
         red_channel_client_pipe_remove_and_release(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
-        pipe_item = &image->link;
+        pipe_item = &image->base;
     }
 }
 
@@ -1949,7 +1949,7 @@ static void red_marshall_image(RedChannelClient *rcc, SpiceMarshaller *m, ImageI
     chunks = spice_chunks_new_linear(item->data, bitmap.stride * bitmap.y);
     bitmap.data = chunks;
 
-    red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_DRAW_COPY, &item->link);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_DISPLAY_DRAW_COPY, &item->base);
 
     copy.base.surface_id = item->surface_id;
     copy.base.box.left = item->pos.x;
diff --git a/server/dcc.c b/server/dcc.c
index c952042..4e450b5 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -184,9 +184,8 @@ ImageItem *dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
 
     item = (ImageItem *)spice_malloc_n_m(height, stride, sizeof(ImageItem));
 
-    pipe_item_init(&item->link, PIPE_ITEM_TYPE_IMAGE);
+    pipe_item_init(&item->base, PIPE_ITEM_TYPE_IMAGE);
 
-    item->refs = 1;
     item->surface_id = surface_id;
     item->image_format =
         spice_bitmap_from_surface_type(surface->context.format);
@@ -214,9 +213,9 @@ ImageItem *dcc_add_surface_area_image(DisplayChannelClient *dcc, int surface_id,
     }
 
     if (pos) {
-        red_channel_client_pipe_add_after(RED_CHANNEL_CLIENT(dcc), &item->link, pos);
+        red_channel_client_pipe_add_after(RED_CHANNEL_CLIENT(dcc), &item->base, pos);
     } else {
-        red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &item->link);
+        red_channel_client_pipe_add(RED_CHANNEL_CLIENT(dcc), &item->base);
     }
 
     return item;
@@ -1591,14 +1590,6 @@ int dcc_handle_migrate_data(DisplayChannelClient *dcc, uint32_t size, void *mess
     return TRUE;
 }
 
-static void image_item_unref(ImageItem *item)
-{
-    if (--item->refs != 0)
-        return;
-
-    free(item);
-}
-
 static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item)
 {
     if (--item->refs != 0)
@@ -1624,7 +1615,7 @@ static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item)
         upgrade_item_unref(display, (UpgradeItem *)item);
         break;
     case PIPE_ITEM_TYPE_IMAGE:
-        image_item_unref((ImageItem *)item);
+        pipe_item_unref(item);
         break;
     case PIPE_ITEM_TYPE_GL_SCANOUT:
     case PIPE_ITEM_TYPE_GL_DRAW:
@@ -1674,7 +1665,7 @@ static void release_item_before_push(DisplayChannelClient *dcc, PipeItem *item)
         upgrade_item_unref(display, (UpgradeItem *)item);
         break;
     case PIPE_ITEM_TYPE_IMAGE:
-        image_item_unref((ImageItem *)item);
+        pipe_item_unref(item);
         break;
     case PIPE_ITEM_TYPE_CREATE_SURFACE: {
         SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem,
diff --git a/server/dcc.h b/server/dcc.h
index 071a9fc..3d7870b 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -136,8 +136,7 @@ typedef struct GlDrawItem {
 } GlDrawItem;
 
 typedef struct ImageItem {
-    PipeItem link;
-    int refs;
+    PipeItem base;
     SpicePoint pos;
     int width;
     int height;
diff --git a/server/display-channel.c b/server/display-channel.c
index a6d90cf..ac10724 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1980,7 +1980,7 @@ static void hold_item(RedChannelClient *rcc, PipeItem *item)
         ((UpgradeItem *)item)->refs++;
         break;
     case PIPE_ITEM_TYPE_IMAGE:
-        ((ImageItem *)item)->refs++;
+        pipe_item_ref(item);
         break;
     default:
         spice_warn_if_reached();
commit 75a2873c6b8e906aabb7e8302fcaaff977887f64
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 7 17:11:21 2016 -0500

    SpiceVmcPipeItem: use base PipeItem for ref counting
    
    This allows to reuse pipe_item_{ref, unref} rather than
    reimplementing them in spicevmc.c

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 1c41845..246886c 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -44,7 +44,6 @@
 
 typedef struct SpiceVmcPipeItem {
     PipeItem base;
-    uint32_t refs;
 
     /* writes which don't fit this will get split, this is not a problem */
     uint8_t buf[BUF_SIZE];
@@ -106,29 +105,16 @@ enum {
     PIPE_ITEM_TYPE_PORT_EVENT,
 };
 
-static SpiceVmcPipeItem *spicevmc_pipe_item_ref(SpiceVmcPipeItem *item)
-{
-    item->refs++;
-    return item;
-}
-
-static void spicevmc_pipe_item_unref(SpiceVmcPipeItem *item)
-{
-    if (!--item->refs) {
-        free(item);
-    }
-}
-
 static RedCharDeviceMsgToClient *spicevmc_chardev_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                                                     void *opaque)
 {
-    return spicevmc_pipe_item_ref((SpiceVmcPipeItem *)msg);
+    return pipe_item_ref(msg);
 }
 
 static void spicevmc_chardev_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                                  void *opaque)
 {
-    spicevmc_pipe_item_unref((SpiceVmcPipeItem *)msg);
+    pipe_item_unref(msg);
 }
 
 static RedCharDeviceMsgToClient *spicevmc_chardev_read_msg_from_dev(SpiceCharDeviceInstance *sin,
@@ -147,7 +133,6 @@ static RedCharDeviceMsgToClient *spicevmc_chardev_read_msg_from_dev(SpiceCharDev
 
     if (!state->pipe_item) {
         msg_item = spice_new0(SpiceVmcPipeItem, 1);
-        msg_item->refs = 1;
         pipe_item_init(&msg_item->base, PIPE_ITEM_TYPE_SPICEVMC_DATA);
     } else {
         spice_assert(state->pipe_item->buf_used == 0);
@@ -175,8 +160,8 @@ static void spicevmc_chardev_send_msg_to_client(RedCharDeviceMsgToClient *msg,
     SpiceVmcPipeItem *vmc_msg = msg;
 
     spice_assert(state->rcc->client == client);
-    spicevmc_pipe_item_ref(vmc_msg);
-    red_channel_client_pipe_add_push(state->rcc, &vmc_msg->base);
+    pipe_item_ref(vmc_msg);
+    red_channel_client_pipe_add_push(state->rcc, (PipeItem *)vmc_msg);
 }
 
 static void spicevmc_port_send_init(RedChannelClient *rcc)
@@ -472,7 +457,7 @@ static void spicevmc_red_channel_release_pipe_item(RedChannelClient *rcc,
     PipeItem *item, int item_pushed)
 {
     if (item->type == PIPE_ITEM_TYPE_SPICEVMC_DATA) {
-        spicevmc_pipe_item_unref((SpiceVmcPipeItem *)item);
+        pipe_item_unref(item);
     } else {
         free(item);
     }
commit eee63f9e28a5a3f670e95a05822031fdf9215054
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 7 17:11:20 2016 -0500

    smartcard: Use base PipeItem for MsgItem refcounting
    
    This allows to reuse pipe_item_{ref, unref} rather than
    reimplementing them in smartcard.c

diff --git a/server/smartcard.c b/server/smartcard.c
index ba6f2f5..76b07f0 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -90,14 +90,11 @@ typedef struct ErrorItem {
 
 typedef struct MsgItem {
     PipeItem base;
-    uint32_t refs;
 
     VSCMsgHeader* vheader;
 } MsgItem;
 
 static MsgItem *smartcard_get_vsc_msg_item(RedChannelClient *rcc, VSCMsgHeader *vheader);
-static MsgItem *smartcard_ref_vsc_msg_item(MsgItem *item);
-static void smartcard_unref_vsc_msg_item(MsgItem *item);
 static void smartcard_channel_client_pipe_add_push(RedChannelClient *rcc, PipeItem *item);
 
 typedef struct SmartCardChannel {
@@ -172,13 +169,13 @@ static RedCharDeviceMsgToClient *smartcard_read_msg_from_device(SpiceCharDeviceI
 static RedCharDeviceMsgToClient *smartcard_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                                              void *opaque)
 {
-    return smartcard_ref_vsc_msg_item((MsgItem *)msg);
+    return pipe_item_ref(msg);
 }
 
 static void smartcard_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                           void *opaque)
 {
-    smartcard_unref_vsc_msg_item((MsgItem *)msg);
+    pipe_item_ref(msg);
 }
 
 static void smartcard_send_msg_to_client(RedCharDeviceMsgToClient *msg,
@@ -187,7 +184,7 @@ static void smartcard_send_msg_to_client(RedCharDeviceMsgToClient *msg,
 {
     RedCharDeviceSmartcard *dev = opaque;
     spice_assert(dev->priv->scc && dev->priv->scc->base.client == client);
-    smartcard_channel_client_pipe_add_push(&dev->priv->scc->base, &((MsgItem *)msg)->base);
+    smartcard_channel_client_pipe_add_push(&dev->priv->scc->base, (PipeItem *)msg);
 
 }
 
@@ -524,7 +521,7 @@ static void smartcard_channel_release_pipe_item(RedChannelClient *rcc,
                                       PipeItem *item, int item_pushed)
 {
     if (item->type == PIPE_ITEM_TYPE_SMARTCARD_DATA) {
-        smartcard_unref_vsc_msg_item((MsgItem *)item);
+        pipe_item_unref(item);
     } else {
         free(item);
     }
@@ -564,30 +561,22 @@ static void smartcard_push_error(RedChannelClient *rcc, uint32_t reader_id, VSCE
     smartcard_channel_client_pipe_add_push(rcc, &error_item->base);
 }
 
+static void smartcard_free_vsc_msg_item(MsgItem *item)
+{
+    free(item->vheader);
+    free(item);
+}
+
 static MsgItem *smartcard_get_vsc_msg_item(RedChannelClient *rcc, VSCMsgHeader *vheader)
 {
     MsgItem *msg_item = spice_new0(MsgItem, 1);
 
-    pipe_item_init(&msg_item->base, PIPE_ITEM_TYPE_SMARTCARD_DATA);
-    msg_item->refs = 1;
+    pipe_item_init_full(&msg_item->base, PIPE_ITEM_TYPE_SMARTCARD_DATA,
+                        (GDestroyNotify)smartcard_free_vsc_msg_item);
     msg_item->vheader = vheader;
     return msg_item;
 }
 
-static MsgItem *smartcard_ref_vsc_msg_item(MsgItem *item)
-{
-    item->refs++;
-    return item;
-}
-
-static void smartcard_unref_vsc_msg_item(MsgItem *item)
-{
-    if (!--item->refs) {
-        free(item->vheader);
-        free(item);
-    }
-}
-
 static void smartcard_remove_reader(SmartCardChannelClient *scc, uint32_t reader_id)
 {
     SpiceCharDeviceInstance *char_device = smartcard_readers_get(reader_id);
commit cc47d9eba84d33012da660d7dcee3a6e3eb8d99f
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 7 17:11:17 2016 -0500

    reds: Derive VDIPortReadBuf from PipeItem
    
    Since PipeItem is already refcounted, this allows to remove various
    layers of ref/unref helpers from reds.c, and use the generic
    pipe_item_{ref, unref} instead.

diff --git a/server/reds.c b/server/reds.c
index f32f3d2..521bc84 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -167,9 +167,8 @@ struct ChannelSecurityOptions {
 };
 
 typedef struct VDIReadBuf {
+    PipeItem parent;
     RedCharDeviceVDIPort *dev;
-    RingItem link;
-    uint32_t refs;
 
     int len;
     uint8_t data[SPICE_AGENT_MAX_DATA_SIZE];
@@ -264,8 +263,7 @@ static uint32_t reds_qxl_ram_size(RedsState *reds);
 static int calc_compression_level(RedsState *reds);
 
 static VDIReadBuf *vdi_port_get_read_buf(RedCharDeviceVDIPort *dev);
-static VDIReadBuf *vdi_port_read_buf_ref(VDIReadBuf *buf);
-static void vdi_port_read_buf_unref(VDIReadBuf *buf);
+static void vdi_port_read_buf_free(VDIReadBuf *buf);
 
 static ChannelSecurityOptions *reds_find_channel_security(RedsState *reds, int id)
 {
@@ -485,7 +483,7 @@ static void reds_reset_vdp(RedsState *reds)
     dev->priv->receive_len = sizeof(dev->priv->vdi_chunk_header);
     dev->priv->message_receive_len = 0;
     if (dev->priv->current_read_buf) {
-        vdi_port_read_buf_unref(dev->priv->current_read_buf);
+        pipe_item_unref(dev->priv->current_read_buf);
         dev->priv->current_read_buf = NULL;
     }
     /* Reset read filter to start with clean state when the agent reconnects */
@@ -704,15 +702,11 @@ static void reds_agent_remove(RedsState *reds)
     }
 }
 
-/*******************************
- * Char device state callbacks *
- * *****************************/
-
 static void vdi_port_read_buf_release(uint8_t *data, void *opaque)
 {
     VDIReadBuf *buf = (VDIReadBuf *)opaque;
 
-    vdi_port_read_buf_unref(buf);
+    pipe_item_unref(buf);
 }
 
 /* returns TRUE if the buffer can be forwarded */
@@ -745,6 +739,16 @@ static gboolean vdi_port_read_buf_process(RedCharDeviceVDIPort *dev, VDIReadBuf
     }
 }
 
+static void vdi_read_buf_init(VDIReadBuf *buf)
+{
+    g_return_if_fail(buf != NULL);
+    /* Bogus pipe item type, we only need the RingItem and refcounting
+     * from the base class and are not going to use the type
+     */
+    pipe_item_init_full(&buf->parent, -1,
+                        (GDestroyNotify)vdi_port_read_buf_free);
+}
+
 static VDIReadBuf *vdi_port_get_read_buf(RedCharDeviceVDIPort *dev)
 {
     RingItem *item;
@@ -755,32 +759,25 @@ static VDIReadBuf *vdi_port_get_read_buf(RedCharDeviceVDIPort *dev)
     }
 
     ring_remove(item);
-    buf = SPICE_CONTAINEROF(item, VDIReadBuf, link);
+    buf = SPICE_CONTAINEROF(item, VDIReadBuf, parent.link);
 
-    buf->refs = 1;
-    return buf;
-}
+    g_warn_if_fail(buf->parent.refcount == 0);
+    vdi_read_buf_init(buf);
 
-static VDIReadBuf* vdi_port_read_buf_ref(VDIReadBuf *buf)
-{
-    buf->refs++;
     return buf;
 }
 
-/* FIXME: refactor so that unreffing the VDIReadBuf doesn't require accessing
- * RedsState? */
-static void vdi_port_read_buf_unref(VDIReadBuf *buf)
+static void vdi_port_read_buf_free(VDIReadBuf *buf)
 {
-    if (!--buf->refs) {
-        ring_add(&buf->dev->priv->read_bufs, &buf->link);
+    g_warn_if_fail(buf->parent.refcount == 0);
+    ring_add(&buf->dev->priv->read_bufs, (RingItem *)buf);
 
-        /* read_one_msg_from_vdi_port may have never completed because the read_bufs
-        ring was empty. So we call it again so it can complete its work if
-        necessary. Note that since we can be called from red_char_device_wakeup
-        this can cause recursion, but we have protection for that */
-        if (buf->dev->priv->agent_attached) {
-            red_char_device_wakeup(RED_CHAR_DEVICE(buf->dev));
-        }
+    /* read_one_msg_from_vdi_port may have never completed because the read_bufs
+       ring was empty. So we call it again so it can complete its work if
+       necessary. Note that since we can be called from red_char_device_wakeup
+       this can cause recursion, but we have protection for that */
+    if (buf->dev->priv->agent_attached) {
+       red_char_device_wakeup(RED_CHAR_DEVICE(buf->dev));
     }
 }
 
@@ -850,7 +847,7 @@ static RedCharDeviceMsgToClient *vdi_port_read_one_msg_from_device(SpiceCharDevi
                 if (error) {
                     reds_agent_remove(reds);
                 }
-                vdi_port_read_buf_unref(dispatch_buf);
+                pipe_item_unref(dispatch_buf);
             }
         }
         } /* END switch */
@@ -861,13 +858,13 @@ static RedCharDeviceMsgToClient *vdi_port_read_one_msg_from_device(SpiceCharDevi
 static RedCharDeviceMsgToClient *vdi_port_ref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                                             void *opaque)
 {
-    return vdi_port_read_buf_ref(msg);
+    return pipe_item_ref(msg);
 }
 
 static void vdi_port_unref_msg_to_client(RedCharDeviceMsgToClient *msg,
                                          void *opaque)
 {
-    vdi_port_read_buf_unref(msg);
+    pipe_item_unref(msg);
 }
 
 /* after calling this, we unref the message, and the ref is in the instance side */
@@ -877,11 +874,12 @@ static void vdi_port_send_msg_to_client(RedCharDeviceMsgToClient *msg,
 {
     VDIReadBuf *agent_data_buf = msg;
 
+    pipe_item_ref(agent_data_buf);
     main_channel_client_push_agent_data(red_client_get_main(client),
                                         agent_data_buf->data,
                                         agent_data_buf->len,
                                         vdi_port_read_buf_release,
-                                        vdi_port_read_buf_ref(agent_data_buf));
+                                        agent_data_buf);
 }
 
 static void vdi_port_send_tokens_to_client(RedClient *client, uint32_t tokens, void *opaque)
@@ -1231,7 +1229,7 @@ void reds_on_main_channel_migrate(RedsState *reds, MainChannelClient *mcc)
             if (error) {
                reds_agent_remove(reds);
             }
-            vdi_port_read_buf_unref(read_buf);
+            pipe_item_unref(read_buf);
         }
 
         spice_assert(agent_dev->priv->receive_len);
@@ -4302,9 +4300,13 @@ red_char_device_vdi_port_init(RedCharDeviceVDIPort *self)
 
     for (i = 0; i < REDS_VDI_PORT_NUM_RECEIVE_BUFFS; i++) {
         VDIReadBuf *buf = spice_new0(VDIReadBuf, 1);
+        vdi_read_buf_init(buf);
         buf->dev = self;
-        ring_item_init(&buf->link);
-        ring_add(&self->priv->read_bufs, &buf->link);
+        g_warn_if_fail(!self->priv->agent_attached);
+        /* This ensures the newly created buffer is placed in the
+         * RedCharDeviceVDIPort::read_bufs queue ready to be reused
+         */
+        pipe_item_unref(buf);
     }
 }
 
commit c1b7f67b820d9070281af4ea24a9b3b3fbf84cfb
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 14 10:54:05 2016 +0100

    Add reference counting to PipeItem class
    
    A user-defined callback is called when the refcount drops to 0.
    
    Reference counting is manually coded for several classes deriving from
    PipeItem, so this change will help to share this code, and allow to remove
    some ref/unref virtual functions in some interfaces when we can assume
    every instance derives from this base class.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/Makefile.am b/server/Makefile.am
index a7a8d9f..a119c86 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -112,6 +112,8 @@ libserver_la_SOURCES =				\
 	display-channel.h			\
 	cursor-channel.c			\
 	cursor-channel.h			\
+	red-pipe-item.c				\
+	red-pipe-item.h				\
 	reds.c					\
 	reds.h					\
 	reds-private.h				\
diff --git a/server/red-channel.c b/server/red-channel.c
index cfddea0..3e036c9 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -1658,12 +1658,6 @@ void red_channel_client_set_message_serial(RedChannelClient *rcc, uint64_t seria
     rcc->send_data.serial = serial;
 }
 
-void pipe_item_init(PipeItem *item, int type)
-{
-    ring_item_init(&item->link);
-    item->type = type;
-}
-
 static inline gboolean client_pipe_add(RedChannelClient *rcc, PipeItem *item, RingItem *pos)
 {
     spice_assert(rcc && item);
diff --git a/server/red-channel.h b/server/red-channel.h
index 94b09eb..3c762ff 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -33,6 +33,7 @@
 #include "demarshallers.h"
 #include "reds-stream.h"
 #include "stat.h"
+#include "red-pipe-item.h"
 
 #define MAX_SEND_BUFS 1000
 #define CLIENT_ACK_WINDOW 20
@@ -147,16 +148,6 @@ enum {
     PIPE_ITEM_TYPE_CHANNEL_BASE=101,
 };
 
-typedef struct PipeItem {
-    RingItem link;
-    int type;
-} PipeItem;
-
-static inline int pipe_item_is_linked(PipeItem *item)
-{
-    return ring_item_is_linked(&item->link);
-}
-
 typedef uint8_t *(*channel_alloc_msg_recv_buf_proc)(RedChannelClient *channel,
                                                     uint16_t type, uint32_t size);
 typedef int (*channel_handle_parsed_proc)(RedChannelClient *rcc, uint32_t size, uint16_t type,
@@ -473,8 +464,6 @@ int red_channel_client_get_roundtrip_ms(RedChannelClient *rcc);
  */
 void red_channel_client_start_connectivity_monitoring(RedChannelClient *rcc, uint32_t timeout_ms);
 
-void pipe_item_init(PipeItem *item, int type);
-
 // TODO: add back the channel_pipe_add functionality - by adding reference counting
 // to the PipeItem.
 
diff --git a/server/red-pipe-item.c b/server/red-pipe-item.c
new file mode 100644
index 0000000..098f1d4
--- /dev/null
+++ b/server/red-pipe-item.c
@@ -0,0 +1,53 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2016 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#include <config.h>
+
+#include "red-channel.h"
+#include "red-pipe-item.h"
+
+PipeItem *pipe_item_ref(gpointer object)
+{
+    PipeItem *item = object;
+
+    g_return_val_if_fail(item->refcount > 0, NULL);
+
+    g_atomic_int_inc(&item->refcount);
+
+    return item;
+}
+
+void pipe_item_unref(gpointer object)
+{
+    PipeItem *item = object;
+
+    g_return_if_fail(item->refcount > 0);
+
+    if (g_atomic_int_dec_and_test(&item->refcount)) {
+        item->free_func(item);
+    }
+}
+
+void pipe_item_init_full(PipeItem *item,
+                         gint type,
+                         GDestroyNotify free_func)
+{
+    ring_item_init(&item->link);
+    item->type = type;
+    item->refcount = 1;
+    item->free_func = free_func ? free_func : (GDestroyNotify)free;
+}
diff --git a/server/red-pipe-item.h b/server/red-pipe-item.h
new file mode 100644
index 0000000..4333e19
--- /dev/null
+++ b/server/red-pipe-item.h
@@ -0,0 +1,46 @@
+/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+   Copyright (C) 2016 Red Hat, Inc.
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with this library; if not, see <http://www.gnu.org/licenses/>.
+*/
+#ifndef _H_RED_PIPE_ITEM
+#define _H_RED_PIPE_ITEM
+
+#include <glib.h>
+
+typedef struct {
+    RingItem link;
+    int type;
+
+    /* private */
+    int refcount;
+
+    GDestroyNotify free_func;
+} PipeItem;
+
+void pipe_item_init_full(PipeItem *item, int type, GDestroyNotify free_func);
+PipeItem *pipe_item_ref(gpointer item);
+void pipe_item_unref(gpointer item);
+
+static inline int pipe_item_is_linked(PipeItem *item)
+{
+    return ring_item_is_linked(&item->link);
+}
+
+static inline void pipe_item_init(PipeItem *item, int type)
+{
+    pipe_item_init_full(item, type, NULL);
+}
+#endif


More information about the Spice-commits mailing list