[Spice-devel] [spice-server 05/10] More removal of IncomingHandlerInterface vfuncs
Frediano Ziglio
fziglio at redhat.com
Fri Feb 10 13:23:59 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.
See my comments on 00/10 and 03/10.
> > ---
> > 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
>
+1, it's not clear the intention of the FIXME (for me patch
looks good without the FIXMEs).
Frediano
> > 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