[Spice-devel] [PATCH spice-gtk 4/9] spice_msg_out[_send_internal]: Take ownership of the passed SpiceMsgOut

Christophe Fergeau cfergeau at redhat.com
Thu Dec 15 09:42:57 PST 2011


On Wed, Dec 14, 2011 at 03:14:04PM +0100, Hans de Goede wrote:
> 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.

Too bad we still need to use spice_msg_out_unref in the smartcard channel,
would have been nice to be able to make it static. Looks good to me
otherwise.

Christophe

> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  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 |    3 ---
>  gtk/channel-usbredir.c  |    1 -
>  gtk/spice-channel.c     |    4 +++-
>  8 files changed, 3 insertions(+), 28 deletions(-)
> 
> 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;
>          }
>      }
> -- 
> 1.7.7.4
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20111215/0d4777bd/attachment.pgp>


More information about the Spice-devel mailing list