[Spice-devel] [PATCH spice-server] red-stream: Remove AsyncRead::stream

[Frediano Ziglio]

> 
> On Tue, Jan 09, 2018 at 07:48:34PM +0000, Frediano Ziglio wrote:
> > AsyncRead is always included in RedStream and there are only
> > a possible operation pending on a RedStream.
> 
> Yes, it's a bit odd that AsyncRead is attached to RedStream, which means
> only one async operation at a time is possible. One could argue it's
> better to keep things asthey are now in case we want to make more
> extensive use of async operations in the future, but I suspect we'd
> have to do more work on RedStream anyway to allow that.
> 

On the other hand you could have only one active attached to a RedStream,
maximum you can have a chain but looking on how is defined would be better
to use a readv instead.

> > @@ -550,13 +549,16 @@ void red_stream_async_read(RedStream *stream,
> >  {
> >      AsyncRead *async = &stream->priv->async_read;
> >  
> > -    g_return_if_fail(!async->stream);
> > -    async->stream = stream;
> > +    g_return_if_fail(async->now == NULL && async->end == NULL);
> 
> > +    if (size == 0) {
> > +        read_done_cb(opaque);
> > +        return;
> > +    }
> 
> This bit seems unrelated?
> 

Yes, is handling a particular case (reading 0 bytes) directly instead of
adding a pending operation. It can be useful if you have a sequence length+data
where length can be 0. Currently cannot happen (size is always > 0), in
red_sasl_handle_auth_steplen where you have a sequence like that you handle
the size == 0 case explicitly with a "if (len == 0) {".

But you are right, is not part of the rationale although IMHO should be a
handled case. I'll remove from the patch.

> Apart from this,
> Acked-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> Christophe
> 

Frediano