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;
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel