[Spice-devel] [spice-server 04/10] Remove IncomingHandlerInterface::{alloc, release}_msg_buf

[Jonathon Jongsma]

On Tue, 2017-02-07 at 11:59 +0100, Christophe Fergeau wrote:
> Similarly to the previous commits, this removes an indirection level,
> IncomingHandlerInterface stores pointers to
> alloc_recv_buf/release_recv_buf vfuncs, but these are always set to
> RedChannel::alloc_recv_buf/RedChannel::release_recv_buf, which are
> also
> vfuncs which can be overridden if needed. This commit removes the
> indirection and directly calls the relevant methods.
> 
> Not sure whether the corresponding vfuncs belong in
> RedChannel or if they should be moved to RedChannelClient.

They do seem more appropriate to RedChannelClient, since the first
argument is RedChannelClient*. If we do move them, it could be done in
a subsequent commit, though. Maybe more of this stuff should be moved
to the client?

> ---
>  server/red-channel-client.c | 40
> ++++++++++++++++++++++++++++++++++++----
>  server/red-channel.c        |  4 ----
>  server/red-channel.h        |  2 --
>  3 files changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index a5c8e9f..a617582 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -1000,6 +1000,33 @@ void
> red_channel_client_shutdown(RedChannelClient *rcc)
>      }
>  }
>  
> +static uint8_t *red_channel_client_alloc_msg_buf(RedChannelClient
> *client,
> +                                                 uint16_t type,
> uint32_t size)
> +{
> +    RedChannel *channel = red_channel_client_get_channel(client);
> +    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +
> +    if (klass->alloc_recv_buf) {
> +        return klass->alloc_recv_buf(client, type, size);
> +    }
> +
> +    g_return_val_if_reached(NULL);
> +}
> +
> +static void red_channel_client_release_msg_buf(RedChannelClient
> *client,
> +                                               uint16_t type,
> uint32_t size,
> +                                               uint8_t *msg)
> +{
> +    RedChannel *channel = red_channel_client_get_channel(client);
> +    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +
> +    if (klass->release_recv_buf) {
> +        klass->release_recv_buf(client, type, size, msg);
> +    }
> +
> +    g_return_if_reached();

It seems that this will trigger every time? Probably a copy-paste error
from the alloc_ function?

> +}
> +
>  static void red_peer_handle_outgoing(RedsStream *stream,
> OutgoingHandler *handler)
>  {
>      ssize_t n;
> @@ -1127,7 +1154,7 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>          msg_type = handler->header.get_msg_type(&handler->header);
>          if (handler->msg_pos < msg_size) {
>              if (!handler->msg) {
> -                handler->msg = handler->cb->alloc_msg_buf(handler-
> >opaque, msg_type, msg_size);
> +                handler->msg =
> red_channel_client_alloc_msg_buf(handler->opaque, msg_type,
> msg_size);
>                  if (handler->msg == NULL) {
>                      spice_printerr("ERROR: channel refused to
> allocate buffer.");
>                      handler->cb->on_error(handler->opaque);
> @@ -1139,7 +1166,8 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>                                            handler->msg + handler-
> >msg_pos,
>                                            msg_size - handler-
> >msg_pos);
>              if (bytes_read == -1) {
> -                handler->cb->release_msg_buf(handler->opaque,
> msg_type, msg_size, handler->msg);
> +                red_channel_client_release_msg_buf(handler->opaque,
> msg_type, msg_size,
> +                                                   handler->msg);
>                  handler->cb->on_error(handler->opaque);
>                  return;
>              }
> @@ -1156,7 +1184,9 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>                  SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
>              if (parsed == NULL) {
>                  spice_printerr("failed to parse message type %d",
> msg_type);
> -                handler->cb->release_msg_buf(handler->opaque,
> msg_type, msg_size, handler->msg);
> +                red_channel_client_release_msg_buf(handler->opaque,
> +                                                   msg_type,
> msg_size,
> +                                                   handler->msg);
>                  handler->cb->on_error(handler->opaque);
>                  return;
>              }
> @@ -1168,7 +1198,9 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>                                                       handler->msg);
>          }
>          handler->msg_pos = 0;
> -        handler->cb->release_msg_buf(handler->opaque, msg_type,
> msg_size, handler->msg);
> +        red_channel_client_release_msg_buf(handler->opaque,
> +                                           msg_type, msg_size,
> +                                           handler->msg);
>          handler->msg = NULL;
>          handler->header_pos = 0;
>  
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 3abe9c7..9e2e527 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -224,10 +224,6 @@ red_channel_constructed(GObject *object)
>      spice_assert(klass->handle_migrate_data ||
>                   !(self->priv->migration_flags &
> SPICE_MIGRATE_NEED_DATA_TRANSFER));
>  
> -    self->priv->incoming_cb.alloc_msg_buf =
> -        (alloc_msg_recv_buf_proc)klass->alloc_recv_buf;
> -    self->priv->incoming_cb.release_msg_buf =
> -        (release_msg_recv_buf_proc)klass->release_recv_buf;
>      self->priv->incoming_cb.handle_message =
> (handle_message_proc)klass->handle_message;
>      self->priv->incoming_cb.handle_parsed =
> (handle_parsed_proc)klass->handle_parsed;
>      self->priv->incoming_cb.parser = klass->parser;
> diff --git a/server/red-channel.h b/server/red-channel.h
> index a24330a..38c192c 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -70,9 +70,7 @@ typedef void (*on_input_proc)(void *opaque, int n);
>  
>  typedef struct IncomingHandlerInterface {
>      handle_message_proc handle_message;
> -    alloc_msg_recv_buf_proc alloc_msg_buf;
>      on_incoming_error_proc on_error; // recv error or handle_message
> error
> -    release_msg_recv_buf_proc release_msg_buf; // for errors
>      // The following is an optional alternative to handle_message,
> used if not null
>      spice_parse_channel_func_t parser;
>      handle_parsed_proc handle_parsed;