[Spice-devel] [PATCH v3 1/9] Reset marshaller as soon as a message is sent

[Frediano Ziglio]

> 
> Any data that is added to the marshaller by reference (using e.g.
> spice_marshaller_add_by_ref_full()) is freed during
> spice_marshaller_reset(). But the marshaller is not currently reset
> until we begin to send the next message (in
> red_channel_client_send_item()). This means that the sent message data
> lives longer than expected and can violate some assumptions in other
> parts of the code.
> 
> To make sure that the data is cleaned up right after being sent, I've
> added a reset call to clear_sent_item() and called that function from
> _on_out_msg_done().  This means that _restore_main_sender() no longer
> needs to reset the marshaller, and we no longer need to call
> _reset_sent_item() within _on_out_msg_done() (since this function is
> called from _clear_sent_item()).
> 
> This prepares the way for refactoring
> red_channel_client_init_send_data() to change how we keep the
> RedPipeItem alive while sending a message.

Acked-by: Frediano Ziglio <fziglio at redhat.com>

> ---
>  server/red-channel-client.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index 24b2fb0..a69146a 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -40,6 +40,7 @@
>  
>  static const SpiceDataHeaderOpaque full_header_wrapper;
>  static const SpiceDataHeaderOpaque mini_header_wrapper;
> +static void red_channel_client_clear_sent_item(RedChannelClient *rcc);
>  static void red_channel_client_destroy_remote_caps(RedChannelClient* rcc);
>  static void red_channel_client_initable_interface_init(GInitableIface
>  *iface);
>  
> @@ -550,7 +551,6 @@ static inline void
> red_channel_client_release_sent_item(RedChannelClient *rcc)
>  
>  static void red_channel_client_restore_main_sender(RedChannelClient *rcc)
>  {
> -    spice_marshaller_reset(rcc->priv->send_data.urgent.marshaller);
>      rcc->priv->send_data.marshaller = rcc->priv->send_data.main.marshaller;
>      rcc->priv->send_data.header.data =
>      rcc->priv->send_data.main.header_data;
>      rcc->priv->send_data.item = rcc->priv->send_data.main.item;
> @@ -561,8 +561,6 @@ void red_channel_client_on_out_msg_done(void *opaque)
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
>      int fd;
>  
> -    rcc->priv->send_data.size = 0;
> -
>      if (spice_marshaller_get_fd(rcc->priv->send_data.marshaller, &fd)) {
>          if (reds_stream_send_msgfd(rcc->priv->stream, fd) < 0) {
>              perror("sendfd");
> @@ -575,7 +573,7 @@ void red_channel_client_on_out_msg_done(void *opaque)
>              close(fd);
>      }
>  
> -    red_channel_client_release_sent_item(rcc);
> +    red_channel_client_clear_sent_item(rcc);
>      if (rcc->priv->send_data.blocked) {
>          SpiceCoreInterfaceInternal *core =
>          red_channel_get_core_interface(rcc->priv->channel);
>          rcc->priv->send_data.blocked = FALSE;
> @@ -1592,6 +1590,7 @@ static void
> red_channel_client_clear_sent_item(RedChannelClient *rcc)
>      red_channel_client_release_sent_item(rcc);
>      rcc->priv->send_data.blocked = FALSE;
>      rcc->priv->send_data.size = 0;
> +    spice_marshaller_reset(rcc->priv->send_data.marshaller);
>  }
>  
>  // TODO: again - what is the context exactly? this happens in channel
>  disconnect. but our

Frediano