Hi
----- Original Message -----
> On Thu, Aug 17, 2017 at 03:35:57PM +0200, marcandre.lureau@xxxxxxxxxx wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> >
> > Instead of printing a warning when trying to read from a closed
> > stream. It can happen that libsoup closes the pipe output stream while
> > the input stream had no pending operation, in which case the close is
> > successful and we shouldn't try to read from the stream anymore.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > ---
> > src/channel-webdav.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/channel-webdav.c b/src/channel-webdav.c
> > index 4a246b5..f1b6c2a 100644
> > --- a/src/channel-webdav.c
> > +++ b/src/channel-webdav.c
> > @@ -218,7 +218,7 @@ client_ref(Client *client)
> > return client;
> > }
> >
> > -static void client_start_read(Client *client);
> > +static bool client_start_read(Client *client);
> >
> > static void remove_client(Client *client)
> > {
> > @@ -234,10 +234,9 @@ static void mux_pushed_cb(OutputQueue *q, gpointer
> > user_data)
> > {
> > Client *client = user_data;
> >
> > - if (client->mux.size == 0) {
> > + if (client->mux.size == 0 ||
> > + !client_start_read(client)) {
> > remove_client(client);
> > - } else {
> > - client_start_read(client);
> > }
> >
> > client_unref(client);
> > @@ -280,14 +279,18 @@ end:
> > client_unref(client);
> > }
> >
> > -static void client_start_read(Client *client)
> > +static bool client_start_read(Client *client)
> > {
> > GInputStream *input;
> >
> > input = g_io_stream_get_input_stream(G_IO_STREAM(client->pipe));
> > + if (g_input_stream_is_closed(input)) {
> > + return false;
> > + }
> > g_input_stream_read_async(input, client->mux.buf, MAX_MUX_SIZE,
> > G_PRIORITY_DEFAULT, client->cancellable,
> > server_reply_cb,
> > client_ref(client));
> > + return true;
> > }
> >
> > static void start_demux(SpiceWebdavChannel *self);
> > @@ -361,6 +364,7 @@ static void start_client(SpiceWebdavChannel *self)
> > SoupServer *server;
> > GSocketAddress *addr;
> > GError *error = NULL;
> > + bool started;
> >
> > session = spice_channel_get_session(SPICE_CHANNEL(self));
> > server =
> > phodav_server_get_soup_server(spice_session_get_webdav_server(session));
> > @@ -384,7 +388,8 @@ G_GNUC_END_IGNORE_DEPRECATIONS
> >
> > g_hash_table_insert(c->clients, &client->id, client);
> >
> > - client_start_read(client);
> > + started = client_start_read(client);
> > + g_assert(started);
>
> Isn't it assert too much?
Since the pipe was just created for the client, it should never return false in client_start_read(), because g_input_stream_is_closed() is not possible. For me, assert() is appropriate in this case. I tend to keep return_if/warnings for unexpected errors that we can cope with, not for impossible situations (like failing to alloc, that glib assert on too for ex).
>
> > demux_to_client(client);
> >
> > g_clear_object(&addr);
> > --
> > 2.14.0.1.geff633fa0
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel