[Spice-devel] [spice-server 05/10] More removal of IncomingHandlerInterface vfuncs
Jonathon Jongsma
jjongsma at redhat.com
Wed Feb 8 22:43:09 UTC 2017
On Tue, 2017-02-07 at 11:59 +0100, Christophe Fergeau wrote:
> This commit removes IncomingHandlerInterface::on_error and
> IncomingHandlerInterface::on_input. As with previous commits, these
> vfuncs are always calling the method, and RedChannel::init sets them
> to
> point to RedChannelClient methods, which RedChannelClient is then
> going
> to call indirectly through the IncomingHandlerInterface instance.
>
> This commit changes this to direct calls to the corresponding
> methods.
> ---
> server/red-channel-client.c | 22 ++++++++++++++--------
> server/red-channel-client.h | 2 --
> server/red-channel.c | 10 ----------
> server/red-channel.h | 2 --
> 4 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/server/red-channel-client.c b/server/red-channel-
> client.c
> index a617582..0c43c46 100644
> --- a/server/red-channel-client.c
> +++ b/server/red-channel-client.c
> @@ -384,7 +384,7 @@ static void red_channel_client_on_output(void
> *opaque, int n)
> red_channel_on_output(channel, n);
> }
>
> -void red_channel_client_on_input(void *opaque, int n)
> +static void red_channel_client_on_input(void *opaque, int n)
> {
> RedChannelClient *rcc = opaque;
>
> @@ -1139,10 +1139,12 @@ static void
> red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
> handler->header.data +
> handler->header_pos,
> handler-
> >header.header_size - handler->header_pos);
> if (bytes_read == -1) {
> - handler->cb->on_error(handler->opaque);
> + /* FIXME: handle disconnection in caller */
> + red_channel_client_disconnect(handler->opaque);
I'm a little bit confused by these FIXME comments. The fact that you've
added them as part of this commit implies that they're somehow related
to these changes. But you didn't really change any behavior with this
patch. And what do they mean exactly?
The rest of the code changes look fine to me.
Jonathon
> return;
> }
> - handler->cb->on_input(handler->opaque, bytes_read);
> + /* FIXME: return number of bytes read, and emit signal
> from caller */
> + red_channel_client_on_input(handler->opaque,
> bytes_read);
> handler->header_pos += bytes_read;
>
> if (handler->header_pos != handler->header.header_size)
> {
> @@ -1157,7 +1159,8 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
> 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);
> + /* FIXME: handle disconnection in caller */
> + red_channel_client_disconnect(handler->opaque);
> return;
> }
> }
> @@ -1168,10 +1171,11 @@ static void
> red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
> if (bytes_read == -1) {
> red_channel_client_release_msg_buf(handler->opaque,
> msg_type, msg_size,
> handler->msg);
> - handler->cb->on_error(handler->opaque);
> + /* FIXME: handle disconnection in caller */
> + red_channel_client_disconnect(handler->opaque);
> return;
> }
> - handler->cb->on_input(handler->opaque, bytes_read);
> + red_channel_client_on_input(handler->opaque,
> bytes_read);
> handler->msg_pos += bytes_read;
> if (handler->msg_pos != msg_size) {
> return;
> @@ -1187,7 +1191,8 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
> red_channel_client_release_msg_buf(handler->opaque,
> msg_type,
> msg_size,
> handler->msg);
> - handler->cb->on_error(handler->opaque);
> + /* FIXME: handle disconnection in caller */
> + red_channel_client_disconnect(handler->opaque);
> return;
> }
> ret_handle = handler->cb->handle_parsed(handler->opaque,
> parsed_size,
> @@ -1205,7 +1210,8 @@ static void red_peer_handle_incoming(RedsStream
> *stream, IncomingHandler *handle
> handler->header_pos = 0;
>
> if (!ret_handle) {
> - handler->cb->on_error(handler->opaque);
> + /* FIXME: handle disconnection in caller */
> + red_channel_client_disconnect(handler->opaque);
> return;
> }
> }
> diff --git a/server/red-channel-client.h b/server/red-channel-
> client.h
> index 6b6a61a..fae7cb2 100644
> --- a/server/red-channel-client.h
> +++ b/server/red-channel-client.h
> @@ -175,8 +175,6 @@ void
> red_channel_client_disconnect_if_pending_send(RedChannelClient *rcc);
>
> RedChannel* red_channel_client_get_channel(RedChannelClient *rcc);
>
> -void red_channel_client_on_input(void *opaque, int n);
> -
> 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 9e2e527..0c809b6 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -196,11 +196,6 @@ red_channel_finalize(GObject *object)
> G_OBJECT_CLASS(red_channel_parent_class)->finalize(object);
> }
>
> -static void
> red_channel_client_default_peer_on_error(RedChannelClient *rcc)
> -{
> - red_channel_client_disconnect(rcc);
> -}
> -
> void red_channel_on_output(RedChannel *self, int n)
> {
> #ifdef RED_STATISTICS
> @@ -324,11 +319,6 @@ red_channel_init(RedChannel *self)
> red_channel_set_common_cap(self, SPICE_COMMON_CAP_MINI_HEADER);
> self->priv->thread_id = pthread_self();
>
> - // TODO: send incoming_cb as parameters instead of duplicating?
> - self->priv->incoming_cb.on_error =
> - (on_incoming_error_proc)red_channel_client_default_peer_on_e
> rror;
> - self->priv->incoming_cb.on_input = red_channel_client_on_input;
> -
> self->priv->client_cbs.connect =
> red_channel_client_default_connect;
> self->priv->client_cbs.disconnect =
> red_channel_client_default_disconnect;
> self->priv->client_cbs.migrate =
> red_channel_client_default_migrate;
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 38c192c..e7c1e1f 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -70,11 +70,9 @@ typedef void (*on_input_proc)(void *opaque, int
> n);
>
> typedef struct IncomingHandlerInterface {
> handle_message_proc handle_message;
> - on_incoming_error_proc on_error; // recv error or handle_message
> error
> // The following is an optional alternative to handle_message,
> used if not null
> spice_parse_channel_func_t parser;
> handle_parsed_proc handle_parsed;
> - on_input_proc on_input;
> } IncomingHandlerInterface;
>
> typedef struct RedChannel RedChannel;
More information about the Spice-devel
mailing list