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

[Frediano Ziglio]

> 
> 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.

Frediano