[Spice-devel] [PATCH spice-gtk 3/6] giopipe: return 0 and no error when peer is already closed

[Marc-André Lureau]

Hi

----- Original Message -----
> > 
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > 
> > The peer stream may be closed, but may have failed to close self,
> > because it has pending operations. In this case indicate EOS on IO
> > rather than returning an error. self will be close eventually on
> > dispose.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > ---
> >  src/giopipe.c | 12 ++++++++++--
> >  tests/pipe.c  |  4 +---
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/giopipe.c b/src/giopipe.c
> > index 08aea9a..4e144f2 100644
> > --- a/src/giopipe.c
> > +++ b/src/giopipe.c
> > @@ -98,7 +98,11 @@ pipe_input_stream_read (GInputStream  *stream,
> >  
> >      g_return_val_if_fail(count > 0, -1);
> >  
> > -    if (g_input_stream_is_closed (stream) || self->peer_closed) {
> > +    if (self->peer_closed) {
> > +        return 0;
> > +    }
> > +
> > +    if (g_input_stream_is_closed (stream)) {
> >          g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
> >                               "Stream is already closed");
> >          return -1;
> > @@ -286,7 +290,11 @@ pipe_output_stream_write (GOutputStream  *stream,
> >      PipeInputStream *peer = self->peer;
> >  
> >      //g_debug("write %p :%"G_GSIZE_FORMAT, stream, count);
> > -    if (g_output_stream_is_closed (stream) || self->peer_closed) {
> > +    if (self->peer_closed) {
> > +        return 0;
> > +    }
> > +
> > +    if (g_output_stream_is_closed (stream)) {
> >          g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_CLOSED,
> >                               "Stream is already closed");
> >          return -1;
> > diff --git a/tests/pipe.c b/tests/pipe.c
> > index a25e79b..7961f4f 100644
> > --- a/tests/pipe.c
> > +++ b/tests/pipe.c
> > @@ -209,9 +209,7 @@ readclose_cb(GObject *source, GAsyncResult *result,
> > gpointer user_data)
> >  
> >      nbytes = g_input_stream_read_finish(G_INPUT_STREAM(source), result,
> >      &error);
> >  
> > -    g_assert_cmpint(nbytes, ==, -1);
> > -    g_assert_error(error, G_IO_ERROR, G_IO_ERROR_CLOSED);
> > -    g_clear_error(&error);
> > +    g_assert_cmpint(nbytes, ==, 0);
> >  
> >      g_main_loop_quit (loop);
> >  }
> 
> Does not seem really good for write. If the pipe is closed and you attempt
> to write why the function should return success?
> It could make more sense for read (and is similar to what sockets does).
> Potentially this change could create infinite loops but I don't know
> how they are used.


Good point for write. Ideally, giopipe should behave like GOutputStream/GInputStream convention. g_output_stream_write() says "0 is never returned", so it's definetly something I should fix. g_input_stream_read() says "Zero is returned on end of file (or if count is zero), but never otherwise.".

I'll fix the patch and make a test.

Thanks