[Spice-commits] 8 commits - gtk/channel-base.c gtk/channel-display.c gtk/channel-inputs.c gtk/channel-main.c gtk/channel-record.c gtk/channel-smartcard.c gtk/channel-usbredir.c gtk/spice-channel-priv.h gtk/spice-channel.c

Hans de Goede jwrdegoede at kemper.freedesktop.org
Fri Dec 16 02:50:25 PST 2011


 gtk/channel-base.c       |    4 -
 gtk/channel-display.c    |    1 
 gtk/channel-inputs.c     |   10 ---
 gtk/channel-main.c       |    5 -
 gtk/channel-record.c     |    3 -
 gtk/channel-smartcard.c  |   16 ++---
 gtk/channel-usbredir.c   |    1 
 gtk/spice-channel-priv.h |    4 -
 gtk/spice-channel.c      |  130 ++++++++++++++---------------------------------
 9 files changed, 48 insertions(+), 126 deletions(-)

New commits:
commit d924fff00f71c8de77268befb0d983dd64f5810b
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Dec 1 16:08:59 2011 +0100

    spice-channel: cleanup, remove spice_channel_send_msg()
    
    There are only 2 callers, both of which want it to do a different thing,
    making the callers do this directly allows us to remove
    spice_channel_send_msg(); and gets rid of the weirdness where we've a
    function which can be called in both co-routine and system context.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 9c9e444..b6779ba 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -42,7 +42,7 @@
 #include "gio-coroutine.h"
 
 static void spice_channel_handle_msg(SpiceChannel *channel, SpiceMsgIn *msg);
-static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboolean buffered);
+static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out);
 static void spice_channel_send_link(SpiceChannel *channel);
 static void channel_disconnect(SpiceChannel *channel);
 
@@ -539,8 +539,9 @@ G_GNUC_INTERNAL
 void spice_msg_out_send(SpiceMsgOut *out)
 {
     g_return_if_fail(out != NULL);
+    g_return_if_fail(out->channel != NULL);
 
-    spice_channel_send_msg(out->channel, out, TRUE);
+    g_queue_push_tail(&out->channel->priv->xmit_queue, out);
 
     /* TODO: we currently flush/wakeup immediately all buffered messages */
     spice_channel_wakeup(out->channel);
@@ -552,7 +553,7 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
 {
     g_return_if_fail(out != NULL);
 
-    spice_channel_send_msg(out->channel, out, FALSE);
+    spice_channel_write_msg(out->channel, out);
 }
 
 /* ---------------------------------------------------------------- */
@@ -1567,22 +1568,6 @@ gboolean spice_channel_get_read_only(SpiceChannel *channel)
     return spice_session_get_read_only(channel->priv->session);
 }
 
-/* coroutine context if @buffered is FALSE,
-   system context if @buffered is TRUE */
-static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboolean buffered)
-{
-    SpiceChannelPrivate *c = channel->priv;
-
-    g_return_if_fail(channel != NULL);
-    g_return_if_fail(out != NULL);
-
-    if (buffered) {
-        g_queue_push_tail(&c->xmit_queue, out);
-    } else {
-        spice_channel_write_msg(channel, out);
-    }
-}
-
 /* coroutine context */
 G_GNUC_INTERNAL
 void spice_channel_recv_msg(SpiceChannel *channel,
commit c3d7e14a6f819083401117518734ea3299a3de00
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Dec 1 15:59:44 2011 +0100

    spice-channel: Move read only check to spice_channel_write_msg()
    
    This is a preparation patch for removing spice_channel_send_msg().
    
    Note that this means that buffered writes won't get checked until they are
    actually send by the co-routine.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 08b8d4b..9c9e444 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -690,6 +690,12 @@ static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
     g_return_if_fail(out != NULL);
     g_return_if_fail(channel == out->channel);
 
+    if (out->ro_check &&
+        spice_channel_get_read_only(channel)) {
+        g_warning("Try to send message while read-only. Please report a bug.");
+        return;
+    }
+
     out->header->size =
         spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     data = spice_marshaller_linearize(out->marshaller, 0, &len, &free_data);
@@ -1570,12 +1576,6 @@ static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboo
     g_return_if_fail(channel != NULL);
     g_return_if_fail(out != NULL);
 
-    if (out->ro_check &&
-        spice_channel_get_read_only(channel)) {
-        g_warning("Try to send message while read-only. Please report a bug.");
-        return;
-    }
-
     if (buffered) {
         g_queue_push_tail(&c->xmit_queue, out);
     } else {
commit 67a78d998461bad38e8293f7122ddac8c6fcf1cc
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Dec 1 15:57:25 2011 +0100

    spice-channel: Move setting of header->size to spice_channel_write_msg()
    
    Just a small cleanup patch.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index ed22378..08b8d4b 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -540,8 +540,6 @@ void spice_msg_out_send(SpiceMsgOut *out)
 {
     g_return_if_fail(out != NULL);
 
-    out->header->size =
-        spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     spice_channel_send_msg(out->channel, out, TRUE);
 
     /* TODO: we currently flush/wakeup immediately all buffered messages */
@@ -554,8 +552,6 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
 {
     g_return_if_fail(out != NULL);
 
-    out->header->size =
-        spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     spice_channel_send_msg(out->channel, out, FALSE);
 }
 
@@ -694,6 +690,8 @@ static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
     g_return_if_fail(out != NULL);
     g_return_if_fail(channel == out->channel);
 
+    out->header->size =
+        spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     data = spice_marshaller_linearize(out->marshaller, 0, &len, &free_data);
     /* spice_msg_out_hexdump(out, data, len); */
     spice_channel_write(channel, data, len);
commit 95b516418717d14d54019ac2f5381d122270e2a6
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Dec 1 15:51:50 2011 +0100

    spice-channel: replace the xmit buf by a queue of SpiceOutMsg-s
    
    This has a number of advantages:
    -It significantly simplifies the code
    -It avoids memcpy-ing all the write data one more time
    -It avoids malloc / realloc / free of the xmit buffer
     (this gets replaced by gslice alloc / free for the queue elements)
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-channel-priv.h b/gtk/spice-channel-priv.h
index 8aa0e7c..ed5f3a7 100644
--- a/gtk/spice-channel-priv.h
+++ b/gtk/spice-channel-priv.h
@@ -94,9 +94,7 @@ struct _SpiceChannelPrivate {
     guint                       connect_delayed_id;
 
     struct wait_queue           wait;
-    guint8                      *xmit_buffer;
-    int                         xmit_buffer_capacity;
-    int                         xmit_buffer_size;
+    GQueue                      xmit_queue;
 
     char                        name[16];
     enum spice_channel_state    state;
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 6a21831..ed22378 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -104,6 +104,7 @@ static void spice_channel_init(SpiceChannel *channel)
     c->remote_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
     c->remote_common_caps = g_array_new(FALSE, TRUE, sizeof(guint32));
     spice_channel_set_common_capability(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
+    g_queue_init(&c->xmit_queue);
 }
 
 static void spice_channel_constructed(GObject *gobject)
@@ -545,8 +546,6 @@ void spice_msg_out_send(SpiceMsgOut *out)
 
     /* TODO: we currently flush/wakeup immediately all buffered messages */
     spice_channel_wakeup(out->channel);
-
-    spice_msg_out_unref(out);
 }
 
 /* coroutine context */
@@ -558,7 +557,6 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
     out->header->size =
         spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     spice_channel_send_msg(out->channel, out, FALSE);
-    spice_msg_out_unref(out);
 }
 
 /* ---------------------------------------------------------------- */
@@ -685,6 +683,27 @@ static void spice_channel_write(SpiceChannel *channel, const void *data, size_t
         spice_channel_flush_wire(channel, data, len);
 }
 
+/* coroutine context */
+static void spice_channel_write_msg(SpiceChannel *channel, SpiceMsgOut *out)
+{
+    uint8_t *data;
+    int free_data;
+    size_t len;
+
+    g_return_if_fail(channel != NULL);
+    g_return_if_fail(out != NULL);
+    g_return_if_fail(channel == out->channel);
+
+    data = spice_marshaller_linearize(out->marshaller, 0, &len, &free_data);
+    /* spice_msg_out_hexdump(out, data, len); */
+    spice_channel_write(channel, data, len);
+
+    if (free_data)
+        free(data);
+
+    spice_msg_out_unref(out);
+}
+
 /*
  * Read at least 1 more byte of data straight off the wire
  * into the requested buffer.
@@ -1529,25 +1548,6 @@ error:
 }
 
 /* system context */
-static void spice_channel_buffered_write(SpiceChannel *channel, const void *data, size_t size)
-{
-    SpiceChannelPrivate *c = channel->priv;
-    size_t left;
-
-    left = c->xmit_buffer_capacity - c->xmit_buffer_size;
-    if (left < size) {
-        c->xmit_buffer_capacity += size + 4095;
-        c->xmit_buffer_capacity &= ~4095;
-
-        c->xmit_buffer = g_realloc(c->xmit_buffer, c->xmit_buffer_capacity);
-    }
-
-    memcpy(&c->xmit_buffer[c->xmit_buffer_size], data, size);
-
-    c->xmit_buffer_size += size;
-}
-
-/* system context */
 /* TODO: we currently flush/wakeup immediately all buffered messages */
 G_GNUC_INTERNAL
 void spice_channel_wakeup(SpiceChannel *channel)
@@ -1567,9 +1567,7 @@ gboolean spice_channel_get_read_only(SpiceChannel *channel)
    system context if @buffered is TRUE */
 static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboolean buffered)
 {
-    uint8_t *data;
-    int free_data;
-    size_t len;
+    SpiceChannelPrivate *c = channel->priv;
 
     g_return_if_fail(channel != NULL);
     g_return_if_fail(out != NULL);
@@ -1580,16 +1578,11 @@ static void spice_channel_send_msg(SpiceChannel *channel, SpiceMsgOut *out, gboo
         return;
     }
 
-    data = spice_marshaller_linearize(out->marshaller, 0,
-                                      &len, &free_data);
-    /* spice_msg_out_hexdump(out, data, len); */
-    if (buffered)
-        spice_channel_buffered_write(channel, data, len);
-    else
-        spice_channel_write(channel, data, len);
-
-    if (free_data)
-        free(data);
+    if (buffered) {
+        g_queue_push_tail(&c->xmit_queue, out);
+    } else {
+        spice_channel_write_msg(channel, out);
+    }
 }
 
 /* coroutine context */
@@ -1806,36 +1799,10 @@ void spice_channel_destroy(SpiceChannel *channel)
 static void spice_channel_iterate_write(SpiceChannel *channel)
 {
     SpiceChannelPrivate *c = channel->priv;
+    SpiceMsgOut *out;
 
-    while (c->xmit_buffer_size) {
-        /*
-         * Take ownership of the buffer, so that if spice_channel_write calls
-         * g_io_wait and thus yields to the main context, and that then calls
-         * spice_channel_buffered_write it does not mess with the buffer
-         * being written out.
-         */
-        guint8 *buffer = c->xmit_buffer;
-        int size = c->xmit_buffer_size;
-        int capacity = c->xmit_buffer_capacity;
-
-        c->xmit_buffer = NULL;
-        c->xmit_buffer_size = 0;
-        c->xmit_buffer_capacity = 0;
-
-        spice_channel_write(channel, buffer, size);
-
-        /*
-         * If no write has been done in the mean time, we can return the buffer
-         * so that it can be re-used.
-         */
-        if (c->xmit_buffer == NULL) {
-            c->xmit_buffer = buffer;
-            c->xmit_buffer_capacity = capacity;
-            c->xmit_buffer_size = 0;
-        } else {
-            g_free(buffer);
-        }
-    }
+    while ((out = g_queue_pop_head(&c->xmit_queue)))
+        spice_channel_write_msg(channel, out);
 }
 
 /* coroutine context */
@@ -2215,12 +2182,8 @@ static void channel_disconnect(SpiceChannel *channel)
     c->peer_msg = NULL;
     c->peer_pos = 0;
 
-    if (c->xmit_buffer) {
-        g_free(c->xmit_buffer);
-        c->xmit_buffer = NULL;
-        c->xmit_buffer_size = 0;
-        c->xmit_buffer_capacity = 0;
-    }
+    g_queue_foreach(&c->xmit_queue, (GFunc)spice_msg_out_unref, NULL);
+    g_queue_clear(&c->xmit_queue);
 
     g_array_set_size(c->remote_common_caps, 0);
     g_array_set_size(c->remote_caps, 0);
commit bcc73fece4483bc404c5ea6b83fb447524959a56
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Dec 1 15:42:40 2011 +0100

    Make sure spice_channel_iterate_write() always flushes *all* pending writes
    
    This patch changes 1 line, and fixes 2 bugs in this one line:
    1) We keep the xmit_buffer around for reuse (re-filling) later, so checking
       if we've a xmit_buffer to determine wether we should do a write is wrong,
       instead we should check if the xmit_buffer has data in it.
    2) If the write blocks, and we switch to another co-routine context, this
       context may queue more writes, we check for this and throw away the
       buffer we've been consuming when this happens, since the other context
       will then have allocated a new buffer. But when this happens, we do *not*
       consume the new buffer. So the queued data will not get transmitted
       until the co-routine gets woken up by either another write, or by
       receiving data to read.
       This patch fixes this by changing the if to a while.
    
    Note that the code in question gets completely removed by the next patch in
    this series. As I noticed this while rewriting the code in question, still
    I decided to do a separate patch to fix this to:
    1) Document this issue in the old code
    2) Otherwise the move from the if to the while will happen in the new code
       which may confuse readers of that patch.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 8ea5a3b..6a21831 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -1807,7 +1807,7 @@ static void spice_channel_iterate_write(SpiceChannel *channel)
 {
     SpiceChannelPrivate *c = channel->priv;
 
-    if (c->xmit_buffer) {
+    while (c->xmit_buffer_size) {
         /*
          * Take ownership of the buffer, so that if spice_channel_write calls
          * g_io_wait and thus yields to the main context, and that then calls
commit d957996c9a571df601a5c53a87a4da7a518985af
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Dec 14 14:28:50 2011 +0100

    spice_msg_out[_send_internal]: Take ownership of the passed SpiceMsgOut
    
    All callers of spice_msg_out[_send_internal] unref the message immediately
    after calling spice_msg_out[_send_internal]. This patch changes the
    semantics so that spice_msg_out[_send_internal] takes ownership and it
    is responsible for unref-ing the passed in SpiceMsgOut.
    
    This is a preparation patch for changing the buffered write code
    to use a queue of SpiceMsgOut-s, rather then memcpy the message contents
    into an intermediate buffer.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/channel-base.c b/gtk/channel-base.c
index c689c71..a6a2892 100644
--- a/gtk/channel-base.c
+++ b/gtk/channel-base.c
@@ -35,7 +35,6 @@ void spice_channel_handle_set_ack(SpiceChannel *channel, SpiceMsgIn *in)
     c->message_ack_window = c->message_ack_count = ack->window;
     c->marshallers->msgc_ack_sync(out->marshaller, &sync);
     spice_msg_out_send_internal(out);
-    spice_msg_out_unref(out);
 }
 
 /* coroutine context */
@@ -48,7 +47,6 @@ void spice_channel_handle_ping(SpiceChannel *channel, SpiceMsgIn *in)
 
     c->marshallers->msgc_pong(pong->marshaller, ping);
     spice_msg_out_send_internal(pong);
-    spice_msg_out_unref(pong);
 }
 
 /* coroutine context */
@@ -130,7 +128,6 @@ void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
 
         out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MIGRATE_FLUSH_MARK);
         spice_msg_out_send_internal(out);
-        spice_msg_out_unref(out);
         SPICE_CHANNEL_GET_CLASS(channel)->iterate_write(channel);
     }
     if (mig->flags & SPICE_MIGRATE_NEED_DATA_TRANSFER) {
@@ -148,6 +145,5 @@ void spice_channel_handle_migrate(SpiceChannel *channel, SpiceMsgIn *in)
         out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MIGRATE_DATA);
         spice_marshaller_add(out->marshaller, data->data, data->header.size);
         spice_msg_out_send_internal(out);
-        spice_msg_out_unref(out);
     }
 }
diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index 923281b..120128f 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -701,7 +701,6 @@ static void spice_display_channel_up(SpiceChannel *channel)
     out = spice_msg_out_new(channel, SPICE_MSGC_DISPLAY_INIT);
     out->marshallers->msgc_display_init(out->marshaller, &init);
     spice_msg_out_send_internal(out);
-    spice_msg_out_unref(out);
 }
 
 #define DRAW(type) {                                                    \
diff --git a/gtk/channel-inputs.c b/gtk/channel-inputs.c
index a29c707..ef35065 100644
--- a/gtk/channel-inputs.c
+++ b/gtk/channel-inputs.c
@@ -223,7 +223,6 @@ static void send_position(SpiceInputsChannel *channel)
         return;
 
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /* main context */
@@ -239,7 +238,6 @@ static void send_motion(SpiceInputsChannel *channel)
         return;
 
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /* coroutine context */
@@ -273,13 +271,11 @@ static void inputs_handle_ack(SpiceChannel *channel, SpiceMsgIn *in)
     msg = mouse_motion(SPICE_INPUTS_CHANNEL(channel));
     if (msg) { /* if no motion, msg == NULL */
         spice_msg_out_send_internal(msg);
-        spice_msg_out_unref(msg);
     }
 
     msg = mouse_position(SPICE_INPUTS_CHANNEL(channel));
     if (msg) {
         spice_msg_out_send_internal(msg);
-        spice_msg_out_unref(msg);
     }
 }
 
@@ -417,7 +413,6 @@ void spice_inputs_button_press(SpiceInputsChannel *channel, gint button,
     press.buttons_state = button_state;
     msg->marshallers->msgc_inputs_mouse_press(msg->marshaller, &press);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /**
@@ -465,7 +460,6 @@ void spice_inputs_button_release(SpiceInputsChannel *channel, gint button,
     release.buttons_state = button_state;
     msg->marshallers->msgc_inputs_mouse_release(msg->marshaller, &release);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /**
@@ -498,7 +492,6 @@ void spice_inputs_key_press(SpiceInputsChannel *channel, guint scancode)
                             SPICE_MSGC_INPUTS_KEY_DOWN);
     msg->marshallers->msgc_inputs_key_down(msg->marshaller, &down);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /**
@@ -531,7 +524,6 @@ void spice_inputs_key_release(SpiceInputsChannel *channel, guint scancode)
                             SPICE_MSGC_INPUTS_KEY_UP);
     msg->marshallers->msgc_inputs_key_up(msg->marshaller, &up);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /* main or coroutine context */
@@ -577,7 +569,6 @@ void spice_inputs_set_key_locks(SpiceInputsChannel *channel, guint locks)
         return;
 
     spice_msg_out_send(msg); /* main -> coroutine */
-    spice_msg_out_unref(msg);
 }
 
 /* coroutine context */
@@ -591,5 +582,4 @@ static void spice_inputs_channel_up(SpiceChannel *channel)
 
     msg = set_key_locks(SPICE_INPUTS_CHANNEL(channel), c->locks);
     spice_msg_out_send_internal(msg);
-    spice_msg_out_unref(msg);
 }
diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index b2d44b6..615cdf6 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -717,7 +717,6 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
         c->agent_tokens--;
         out = g_queue_pop_head(c->agent_msg_queue);
         spice_msg_out_send_internal(out);
-        spice_msg_out_unref(out);
     }
 }
 
@@ -1023,7 +1022,6 @@ static void agent_start(SpiceMainChannel *channel)
     out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MAIN_AGENT_START);
     out->marshallers->msgc_main_agent_start(out->marshaller, &agent_start);
     spice_msg_out_send_internal(out);
-    spice_msg_out_unref(out);
 
     if (c->agent_connected) {
         agent_announce_caps(channel);
@@ -1065,7 +1063,6 @@ static void set_mouse_mode(SpiceMainChannel *channel, uint32_t supported, uint32
         out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MAIN_MOUSE_MODE_REQUEST);
         out->marshallers->msgc_main_mouse_mode_request(out->marshaller, &req);
         spice_msg_out_send_internal(out);
-        spice_msg_out_unref(out);
     }
 }
 
@@ -1082,7 +1079,6 @@ static void main_handle_init(SpiceChannel *channel, SpiceMsgIn *in)
 
     out = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_MAIN_ATTACH_CHANNELS);
     spice_msg_out_send_internal(out);
-    spice_msg_out_unref(out);
 
     set_mouse_mode(SPICE_MAIN_CHANNEL(channel), init->supported_mouse_modes,
                    init->current_mouse_mode);
@@ -1508,7 +1504,6 @@ static void main_handle_migrate_begin(SpiceChannel *channel, SpiceMsgIn *in)
 
     out = spice_msg_out_new(SPICE_CHANNEL(channel), reply_type);
     spice_msg_out_send(out);
-    spice_msg_out_unref(out);
 }
 
 /* main context */
diff --git a/gtk/channel-record.c b/gtk/channel-record.c
index bb66c3b..4e5e893 100644
--- a/gtk/channel-record.c
+++ b/gtk/channel-record.c
@@ -288,7 +288,6 @@ static void spice_record_mode(SpiceRecordChannel *channel, uint32_t time,
     msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_RECORD_MODE);
     msg->marshallers->msgc_record_mode(msg->marshaller, &m);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /* coroutine context */
@@ -319,7 +318,6 @@ static void spice_record_start_mark(SpiceRecordChannel *channel, uint32_t time)
     msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_RECORD_START_MARK);
     msg->marshallers->msgc_record_start_mark(msg->marshaller, &m);
     spice_msg_out_send(msg);
-    spice_msg_out_unref(msg);
 }
 
 /**
@@ -398,7 +396,6 @@ void spice_record_send_data(SpiceRecordChannel *channel, gpointer data,
         msg->marshallers->msgc_record_data(msg->marshaller, &p);
         spice_marshaller_add(msg->marshaller, frame, frame_size);
         spice_msg_out_send(msg);
-        spice_msg_out_unref(msg);
 
         if (rc->last_frame_current == rc->frame_bytes)
             rc->last_frame_current = 0;
diff --git a/gtk/channel-smartcard.c b/gtk/channel-smartcard.c
index 1df91ce..98e77eb 100644
--- a/gtk/channel-smartcard.c
+++ b/gtk/channel-smartcard.c
@@ -271,7 +271,6 @@ smartcard_message_complete_in_flight(SpiceSmartcardChannel *channel)
     channel->priv->in_flight_message = g_queue_pop_head(channel->priv->message_queue);
     if (channel->priv->in_flight_message != NULL) {
         spice_msg_out_send(channel->priv->in_flight_message->message);
-        spice_msg_out_unref(channel->priv->in_flight_message->message);
         channel->priv->in_flight_message->message = NULL;
     }
 }
@@ -289,7 +288,6 @@ static void smartcard_message_send(SpiceSmartcardChannel *channel,
                 msg_type, queue ? "queued" : "now");
     if (!queue) {
         spice_msg_out_send(msg_out);
-        spice_msg_out_unref(msg_out);
         return;
     }
 
@@ -298,7 +296,6 @@ static void smartcard_message_send(SpiceSmartcardChannel *channel,
         g_return_if_fail(g_queue_is_empty(channel->priv->message_queue));
         channel->priv->in_flight_message = message;
         spice_msg_out_send(channel->priv->in_flight_message->message);
-        spice_msg_out_unref(channel->priv->in_flight_message->message);
         channel->priv->in_flight_message->message = NULL;
     } else {
         g_queue_push_tail(channel->priv->message_queue, message);
diff --git a/gtk/channel-usbredir.c b/gtk/channel-usbredir.c
index fce4783..06d80d5 100644
--- a/gtk/channel-usbredir.c
+++ b/gtk/channel-usbredir.c
@@ -367,7 +367,6 @@ void spice_usbredir_channel_do_write(SpiceUsbredirChannel *channel)
     usbredirhost_write_guest_data(priv->host);
 
     spice_msg_out_send(priv->msg_out);
-    spice_msg_out_unref(priv->msg_out);
     priv->msg_out = NULL;
 }
 
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index 7a57630..8ea5a3b 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -545,6 +545,8 @@ void spice_msg_out_send(SpiceMsgOut *out)
 
     /* TODO: we currently flush/wakeup immediately all buffered messages */
     spice_channel_wakeup(out->channel);
+
+    spice_msg_out_unref(out);
 }
 
 /* coroutine context */
@@ -556,6 +558,7 @@ void spice_msg_out_send_internal(SpiceMsgOut *out)
     out->header->size =
         spice_marshaller_get_total_size(out->marshaller) - sizeof(SpiceDataHeader);
     spice_channel_send_msg(out->channel, out, FALSE);
+    spice_msg_out_unref(out);
 }
 
 /* ---------------------------------------------------------------- */
@@ -1657,7 +1660,6 @@ void spice_channel_recv_msg(SpiceChannel *channel,
         if (!c->message_ack_count) {
             SpiceMsgOut *out = spice_msg_out_new(channel, SPICE_MSGC_ACK);
             spice_msg_out_send_internal(out);
-            spice_msg_out_unref(out);
             c->message_ack_count = c->message_ack_window;
         }
     }
commit 8de100daf86f0e53ff3ddafd5e7a1d21663d40e2
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Sat Dec 3 14:57:10 2011 +0100

    channel-smartcard: unref in flight messages as soon as they are sent
    
    This is a preparation patch for making spice_msg_out_send() take ownership
    of the passed in msg.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/channel-smartcard.c b/gtk/channel-smartcard.c
index 64fc06e..1df91ce 100644
--- a/gtk/channel-smartcard.c
+++ b/gtk/channel-smartcard.c
@@ -191,7 +191,8 @@ static const spice_msg_handler smartcard_handlers[] = {
 static void
 smartcard_message_free(SpiceSmartcardChannelMessage *message)
 {
-    spice_msg_out_unref(message->message);
+    if (message->message)
+        spice_msg_out_unref(message->message);
     g_slice_free(SpiceSmartcardChannelMessage, message);
 }
 
@@ -268,8 +269,11 @@ smartcard_message_complete_in_flight(SpiceSmartcardChannel *channel)
 
     smartcard_message_free(channel->priv->in_flight_message);
     channel->priv->in_flight_message = g_queue_pop_head(channel->priv->message_queue);
-    if (channel->priv->in_flight_message != NULL)
+    if (channel->priv->in_flight_message != NULL) {
         spice_msg_out_send(channel->priv->in_flight_message->message);
+        spice_msg_out_unref(channel->priv->in_flight_message->message);
+        channel->priv->in_flight_message->message = NULL;
+    }
 }
 
 static void smartcard_message_send(SpiceSmartcardChannel *channel,
@@ -293,7 +297,9 @@ static void smartcard_message_send(SpiceSmartcardChannel *channel,
     if (channel->priv->in_flight_message == NULL) {
         g_return_if_fail(g_queue_is_empty(channel->priv->message_queue));
         channel->priv->in_flight_message = message;
-        spice_msg_out_send(msg_out);
+        spice_msg_out_send(channel->priv->in_flight_message->message);
+        spice_msg_out_unref(channel->priv->in_flight_message->message);
+        channel->priv->in_flight_message->message = NULL;
     } else {
         g_queue_push_tail(channel->priv->message_queue, message);
     }
commit f04475aeb76914edac688c775ddebde279a9e61b
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Wed Dec 14 12:56:57 2011 +0100

    smartcard_message_complete_in_flight cleanup error checking
    
    smartcard_message_complete_in_flight should never get called if there
    is no message in flight (priv->in_flight_message != NULL).
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/channel-smartcard.c b/gtk/channel-smartcard.c
index f376e53..64fc06e 100644
--- a/gtk/channel-smartcard.c
+++ b/gtk/channel-smartcard.c
@@ -264,10 +264,7 @@ smartcard_message_new(VSCMsgType msg_type, SpiceMsgOut *msg_out)
 static void
 smartcard_message_complete_in_flight(SpiceSmartcardChannel *channel)
 {
-    if (channel->priv->in_flight_message == NULL) {
-        g_return_if_fail(g_queue_is_empty(channel->priv->message_queue));
-        return;
-    }
+    g_return_if_fail(channel->priv->in_flight_message != NULL);
 
     smartcard_message_free(channel->priv->in_flight_message);
     channel->priv->in_flight_message = g_queue_pop_head(channel->priv->message_queue);


More information about the Spice-commits mailing list