[Spice-devel] [spice-server v2 05/14] channel: Remove IncomingHandlerInterface::{alloc, release}_msg_buf

[Frediano Ziglio]

> 
> 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.
> 

Didn't we agree on a follow up? So should be

"The corresponding vfuncs should belong to RedChannelClient
and will be moved with future patches"

or something similar ?

> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/red-channel-client.c | 32 ++++++++++++++++++++++++++++----
>  server/red-channel.c        |  4 ----
>  server/red-channel.h        |  5 -----
>  3 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/server/red-channel-client.c b/server/red-channel-client.c
> index 7d64e58..9b7b818 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -1000,6 +1000,25 @@ void red_channel_client_shutdown(RedChannelClient
> *rcc)
>      }
>  }
>  
> +static uint8_t *red_channel_client_alloc_msg_buf(RedChannelClient *rcc,
> +                                                 uint16_t type, uint32_t
> size)
> +{
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +
> +    return klass->alloc_recv_buf(rcc, type, size);
> +}
> +
> +static void red_channel_client_release_msg_buf(RedChannelClient *rcc,
> +                                               uint16_t type, uint32_t size,
> +                                               uint8_t *msg)
> +{
> +    RedChannel *channel = red_channel_client_get_channel(rcc);
> +    RedChannelClass *klass = RED_CHANNEL_GET_CLASS(channel);
> +
> +    klass->release_recv_buf(rcc, type, size, msg);
> +}
> +
>  static void red_peer_handle_outgoing(RedsStream *stream, OutgoingHandler
>  *handler)
>  {
>      ssize_t n;
> @@ -1143,7 +1162,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);
> @@ -1155,7 +1174,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;
>              }
> @@ -1172,7 +1192,9 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>                                            &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;
>          }
> @@ -1182,7 +1204,9 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
>              parsed_free(parsed);
>          }
>          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 f25caba..52902d0 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.parser = klass->parser;
>  }
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 91db904..d373b88 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -61,16 +61,11 @@ struct SpiceDataHeaderOpaque {
>  
>  typedef int (*handle_message_proc)(void *opaque,
>                                     uint16_t type, uint32_t size, uint8_t
>                                     *msg);
> -typedef uint8_t *(*alloc_msg_recv_buf_proc)(void *opaque, uint16_t type,
> uint32_t size);
> -typedef void (*release_msg_recv_buf_proc)(void *opaque,
> -                                          uint16_t type, uint32_t size,
> uint8_t *msg);
>  typedef void (*on_incoming_error_proc)(void *opaque);
>  typedef void (*on_input_proc)(void *opaque, int n);
>  
>  typedef struct IncomingHandlerInterface {
> -    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
>      // 'parser' is optional and will not be used if NULL
>      spice_parse_channel_func_t parser;
>      handle_message_proc handle_message;

Otherwise

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

Frediano