On Wed, Feb 08, 2017 at 04:43:09PM -0600, Jonathon Jongsma wrote:
> 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?
See
https://lists.freedesktop.org/archives/spice-devel/2017-February/035612.html
for explanations
« Ah, this got Jonathon confused later on too, my feeling is that things
would look better as:
int status = red_peer_handle_outgoing(..)
switch (status) {
case 0:
red_channel_client_on_out_msg_done(red_channel_client);
break;
case EPIPE:
default:
red_channel_disconnect(red_channel_client);
break;
}
and limit as much as possible calls to red_channel_client_xxx from
red_peer_handle_outgoing(). Did not explore this yet, so cannot tell
if it makes sense or not :) »
I guess I'll drop these and keep that in the back of my mind ;)
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel