[Spice-devel] [spice-server v2 04/14] channel: Remove OutgoingHandlerInterface

Frediano Ziglio fziglio at redhat.com
Tue Feb 14 14:56:47 UTC 2017


> 
> RedChannel uses OutgoingHandlerInterface to provide constant pointers to
> RedChannelClient methods. This OutgoingHandlerInterface structure is
> then used in RedChannelClient to indirectly call these methods.
> 
> The OutgoingHandlerInterface abstraction is unused, ie the codebase
> only has a single implementation for it, so we can directly call the
> relevant methods and make them static instead.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/red-channel-client-private.h |  1 -
>  server/red-channel-client.c         | 28 ++++++++++++++--------------
>  server/red-channel-client.h         |  6 ------
>  server/red-channel.c                | 13 -------------
>  server/red-channel.h                | 20 +-------------------
>  5 files changed, 15 insertions(+), 53 deletions(-)
> 
> diff --git a/server/red-channel-client-private.h
> b/server/red-channel-client-private.h
> index d01cdbd..5d29f32 100644
> --- a/server/red-channel-client-private.h
> +++ b/server/red-channel-client-private.h
> @@ -41,7 +41,6 @@ typedef struct RedChannelClientConnectivityMonitor {
>  } RedChannelClientConnectivityMonitor;
>  
>  typedef struct OutgoingHandler {
> -    OutgoingHandlerInterface *cb;
>      void *opaque;
>      struct iovec vec_buf[IOV_MAX];
>      int vec_size;
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index cc1c846..7d64e58 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -272,7 +272,6 @@ static void red_channel_client_constructed(GObject
> *object)
>      self->priv->incoming.cb =
>      red_channel_get_incoming_handler(self->priv->channel);
>  
>      self->priv->outgoing.opaque = self;
> -    self->priv->outgoing.cb =
> red_channel_get_outgoing_handler(self->priv->channel);
>      self->priv->outgoing.pos = 0;
>      self->priv->outgoing.size = 0;
>  
> @@ -373,7 +372,7 @@ RedChannel*
> red_channel_client_get_channel(RedChannelClient *rcc)
>      return rcc->priv->channel;
>  }
>  
> -void red_channel_client_on_output(void *opaque, int n)
> +static void red_channel_client_data_sent(void *opaque, int n)
>  {
>      RedChannelClient *rcc = opaque;
>      RedChannel *channel = red_channel_client_get_channel(rcc);
> @@ -381,6 +380,7 @@ void red_channel_client_on_output(void *opaque, int n)
>      if (rcc->priv->connectivity_monitor.timer) {
>          rcc->priv->connectivity_monitor.out_bytes += n;
>      }
> +    /* FIXME: use a signal rather than a hardcoded call to a RedChannel
> callback? */
>      red_channel_on_output(channel, n);
>  }
>  

This comment should go in 03/14.

> @@ -393,15 +393,15 @@ void red_channel_client_on_input(void *opaque, int n)
>      }
>  }
>  
> -int red_channel_client_get_out_msg_size(void *opaque)
> +static int red_channel_client_get_out_msg_size(void *opaque)
>  {
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
>  
>      return rcc->priv->send_data.size;
>  }
>  
> -void red_channel_client_prepare_out_msg(void *opaque, struct iovec *vec,
> -                                        int *vec_size, int pos)
> +static void red_channel_client_prepare_out_msg(void *opaque, struct iovec
> *vec,
> +                                               int *vec_size, int pos)
>  {
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
>  
> @@ -409,7 +409,7 @@ void red_channel_client_prepare_out_msg(void *opaque,
> struct iovec *vec,
>                                              vec, IOV_MAX, pos);
>  }
>  
> -void red_channel_client_on_out_block(void *opaque)
> +static void red_channel_client_set_blocked(void *opaque)
>  {
>      SpiceCoreInterfaceInternal *core;
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
> @@ -549,7 +549,7 @@ static void
> red_channel_client_restore_main_sender(RedChannelClient *rcc)
>      rcc->priv->send_data.header.data =
>      rcc->priv->send_data.main.header_data;
>  }
>  
> -void red_channel_client_on_out_msg_done(void *opaque)
> +static void red_channel_client_msg_sent(void *opaque)
>  {
>      RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
>      int fd;
> @@ -1010,33 +1010,33 @@ static void red_peer_handle_outgoing(RedsStream
> *stream, OutgoingHandler *handle
>  
>      if (handler->size == 0) {
>          handler->vec = handler->vec_buf;
> -        handler->size = handler->cb->get_msg_size(handler->opaque);
> +        handler->size =
> red_channel_client_get_out_msg_size(handler->opaque);
>          if (!handler->size) {  // nothing to be sent
>              return;
>          }
>      }
>  
>      for (;;) {
> -        handler->cb->prepare(handler->opaque, handler->vec,
> &handler->vec_size, handler->pos);
> +        red_channel_client_prepare_out_msg(handler->opaque, handler->vec,
> &handler->vec_size, handler->pos);
>          n = reds_stream_writev(stream, handler->vec, handler->vec_size);
>          if (n == -1) {
>              switch (errno) {
>              case EAGAIN:
> -                handler->cb->on_block(handler->opaque);
> +                red_channel_client_set_blocked(handler->opaque);
>                  return;
>              case EINTR:
>                  continue;
>              case EPIPE:
> -                handler->cb->on_error(handler->opaque);
> +                red_channel_client_disconnect(handler->opaque);
>                  return;
>              default:
>                  spice_printerr("%s", strerror(errno));
> -                handler->cb->on_error(handler->opaque);
> +                red_channel_client_disconnect(handler->opaque);
>                  return;
>              }
>          } else {
>              handler->pos += n;
> -            handler->cb->on_output(handler->opaque, n);
> +            red_channel_client_data_sent(handler->opaque, n);
>              if (handler->pos == handler->size) { // finished writing data
>                  /* reset handler before calling on_msg_done, since it
>                   * can trigger another call to red_peer_handle_outgoing
>                   (when
> @@ -1044,7 +1044,7 @@ static void red_peer_handle_outgoing(RedsStream
> *stream, OutgoingHandler *handle
>                  handler->vec = handler->vec_buf;
>                  handler->pos = 0;
>                  handler->size = 0;
> -                handler->cb->on_msg_done(handler->opaque);
> +                red_channel_client_msg_sent(handler->opaque);
>                  return;
>              }
>          }
> diff --git a/server/red-channel-client.h b/server/red-channel-client.h
> index d54e7dd..9e2bf73 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -175,13 +175,7 @@ void
> red_channel_client_disconnect_if_pending_send(RedChannelClient *rcc);
>  
>  RedChannel* red_channel_client_get_channel(RedChannelClient *rcc);
>  
> -void red_channel_client_on_output(void *opaque, int n);
>  void red_channel_client_on_input(void *opaque, int n);
> -int red_channel_client_get_out_msg_size(void *opaque);
> -void red_channel_client_prepare_out_msg(void *opaque, struct iovec *vec,
> -                                             int *vec_size, int pos);
> -void red_channel_client_on_out_block(void *opaque);
> -void red_channel_client_on_out_msg_done(void *opaque);
>  
>  void red_channel_client_semi_seamless_migration_complete(RedChannelClient
>  *rcc);
>  void red_channel_client_init_outgoing_messages_window(RedChannelClient
>  *rcc);
> diff --git a/server/red-channel.c b/server/red-channel.c
> index b53bbcd..f25caba 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -91,7 +91,6 @@ struct RedChannelPrivate
>      RedChannelCapabilities local_caps;
>      uint32_t migration_flags;
>  
> -    OutgoingHandlerInterface outgoing_cb;
>      IncomingHandlerInterface incoming_cb;
>  
>      ClientCbs client_cbs;
> @@ -332,13 +331,6 @@ red_channel_init(RedChannel *self)
>      self->priv->incoming_cb.on_error =
>          (on_incoming_error_proc)red_channel_client_default_peer_on_error;
>      self->priv->incoming_cb.on_input = red_channel_client_on_input;
> -    self->priv->outgoing_cb.get_msg_size =
> red_channel_client_get_out_msg_size;
> -    self->priv->outgoing_cb.prepare = red_channel_client_prepare_out_msg;
> -    self->priv->outgoing_cb.on_block = red_channel_client_on_out_block;
> -    self->priv->outgoing_cb.on_error =
> -        (on_outgoing_error_proc)red_channel_client_default_peer_on_error;
> -    self->priv->outgoing_cb.on_msg_done =
> red_channel_client_on_out_msg_done;
> -    self->priv->outgoing_cb.on_output = red_channel_client_on_output;
>  
>      self->priv->client_cbs.connect = red_channel_client_default_connect;
>      self->priv->client_cbs.disconnect =
>      red_channel_client_default_disconnect;
> @@ -787,11 +779,6 @@ IncomingHandlerInterface*
> red_channel_get_incoming_handler(RedChannel *self)
>      return &self->priv->incoming_cb;
>  }
>  
> -OutgoingHandlerInterface* red_channel_get_outgoing_handler(RedChannel *self)
> -{
> -    return &self->priv->outgoing_cb;
> -}
> -
>  void red_channel_reset_thread_id(RedChannel *self)
>  {
>      self->priv->thread_id = pthread_self();
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 23374b8..91db904 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -77,23 +77,6 @@ typedef struct IncomingHandlerInterface {
>      on_input_proc on_input;
>  } IncomingHandlerInterface;
>  
> -typedef int (*get_outgoing_msg_size_proc)(void *opaque);
> -typedef void (*prepare_outgoing_proc)(void *opaque, struct iovec *vec, int
> *vec_size, int pos);
> -typedef void (*on_outgoing_error_proc)(void *opaque);
> -typedef void (*on_outgoing_block_proc)(void *opaque);
> -typedef void (*on_outgoing_msg_done_proc)(void *opaque);
> -typedef void (*on_output_proc)(void *opaque, int n);
> -
> -typedef struct OutgoingHandlerInterface {
> -    get_outgoing_msg_size_proc get_msg_size;
> -    prepare_outgoing_proc prepare;
> -    on_outgoing_error_proc on_error;
> -    on_outgoing_block_proc on_block;
> -    on_outgoing_msg_done_proc on_msg_done;
> -    on_output_proc on_output;
> -} OutgoingHandlerInterface;
> -/* Red Channel interface */
> -
>  typedef struct RedChannel RedChannel;
>  typedef struct RedChannelClient RedChannelClient;
>  typedef struct RedClient RedClient;
> @@ -301,10 +284,9 @@ void red_channel_send_item(RedChannel *self,
> RedChannelClient *rcc, RedPipeItem
>  void red_channel_reset_thread_id(RedChannel *self);
>  StatNodeRef red_channel_get_stat_node(RedChannel *channel);
>  
> -/* FIXME: do these even need to be in RedChannel? It's really only used in
> +/* FIXME: does this even need to be in RedChannel? It's really only used in
>   * RedChannelClient. Needs refactoring */
>  IncomingHandlerInterface* red_channel_get_incoming_handler(RedChannel
>  *self);
> -OutgoingHandlerInterface* red_channel_get_outgoing_handler(RedChannel
> *self);
>  
>  const RedChannelCapabilities* red_channel_get_local_capabilities(RedChannel
>  *self);
>  

Otherwise

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

Frediano


More information about the Spice-devel mailing list