Hi, On Mon, May 18, 2015 at 01:33:02PM +0200, Marc-André Lureau wrote: > Hi > > On Mon, May 18, 2015 at 9:09 AM, Victor Toso <victortoso@xxxxxxxxxx> wrote: > > > changes: > > - unecessary return on demux_to_client; > > > > that doesn't hurt, does it? it makes things more explicit perhaps It does not hurt. > > > - client struct has reference to current SpiceWebdavChannel so it can be > > removed as parameter in client functions; > > > > I would rather keep it as argument to the function too, imho it's more > clear like that and more consistant (looks like a regular channel method) > > - using gboolean parameter to check if demux_to_client_finish failed > > will be useful in the next commit > > > > then perhaps it should be part of next commit, as I don't get it here :) the point is that gssize will become gsize in the next commit. It does not make sense to verify gsize <= 0 ... as the value is not used, I thought a boolean would fit better. > Please split the commit for easier review Sure, sorry for that. I'll drop this clean-up patch and have what is necessary in the next patch on v2. > > > --- > > gtk/channel-webdav.c | 44 ++++++++++++++++++++++---------------------- > > 1 file changed, 22 insertions(+), 22 deletions(-) > > > > diff --git a/gtk/channel-webdav.c b/gtk/channel-webdav.c > > index 1d3862e..9e3a824 100644 > > --- a/gtk/channel-webdav.c > > +++ b/gtk/channel-webdav.c > > @@ -219,9 +219,9 @@ client_ref(Client *client) > > return client; > > } > > > > -static void client_start_read(SpiceWebdavChannel *self, Client *client); > > +static void client_start_read(Client *client); > > > > -static void remove_client(SpiceWebdavChannel *self, Client *client) > > +static void remove_client(Client *client) > > { > > SpiceWebdavChannelPrivate *c; > > > > @@ -230,7 +230,7 @@ static void remove_client(SpiceWebdavChannel *self, > > Client *client) > > > > g_cancellable_cancel(client->cancellable); > > > > - c = self->priv; > > + c = client->self->priv; > > g_hash_table_remove(c->clients, &client->id); > > } > > > > @@ -239,9 +239,9 @@ static void mux_pushed_cb(OutputQueue *q, gpointer > > user_data) > > Client *client = user_data; > > > > if (client->mux.size == 0) { > > - remove_client(client->self, client); > > + remove_client(client); > > } else { > > - client_start_read(client->self, client); > > + client_start_read(client); > > } > > > > client_unref(client); > > @@ -278,14 +278,14 @@ end: > > if (err) { > > if (!g_cancellable_is_cancelled(client->cancellable)) > > g_warning("read error: %s", err->message); > > - remove_client(self, client); > > + remove_client(client); > > g_clear_error(&err); > > } > > > > client_unref(client); > > } > > > > -static void client_start_read(SpiceWebdavChannel *self, Client *client) > > +static void client_start_read(Client *client) > > { > > GInputStream *input; > > > > @@ -297,14 +297,13 @@ static void client_start_read(SpiceWebdavChannel > > *self, Client *client) > > > > static void start_demux(SpiceWebdavChannel *self); > > > > -static void demux_to_client_finish(SpiceWebdavChannel *self, > > - Client *client, gssize size) > > +static void demux_to_client_finish(Client *client, gboolean fail) > > { > > + SpiceWebdavChannel *self = client->self; > > SpiceWebdavChannelPrivate *c = self->priv; > > > > - if (size <= 0) { > > - remove_client(self, client); > > - } > > + if (fail) > > + remove_client(client); > > > > c->demuxing = FALSE; > > start_demux(self); > > @@ -315,6 +314,7 @@ static void demux_to_client_cb(GObject *source, > > GAsyncResult *result, gpointer u > > Client *client = user_data; > > SpiceWebdavChannelPrivate *c = client->self->priv; > > GError *error = NULL; > > + gboolean fail; > > gssize size; > > > > size = g_output_stream_write_finish(G_OUTPUT_STREAM(source), result, > > &error); > > @@ -324,25 +324,25 @@ static void demux_to_client_cb(GObject *source, > > GAsyncResult *result, gpointer u > > g_clear_error(&error); > > } > > > > + fail = (size != c->demux.size); > > g_warn_if_fail(size == c->demux.size); > > - demux_to_client_finish(client->self, client, size); > > + demux_to_client_finish(client, fail); > > } > > > > -static void demux_to_client(SpiceWebdavChannel *self, > > - Client *client) > > +static void demux_to_client(Client *client) > > { > > - SpiceWebdavChannelPrivate *c = self->priv; > > + SpiceWebdavChannelPrivate *c = client->self->priv; > > gssize size = c->demux.size; > > > > - CHANNEL_DEBUG(self, "pushing %"G_GSSIZE_FORMAT" to client %p", size, > > client); > > + CHANNEL_DEBUG(client->self, "pushing %"G_GSSIZE_FORMAT" to client > > %p", size, client); > > > > if (size > 0) { > > > > g_output_stream_write_async(g_io_stream_get_output_stream(client->pipe), > > c->demux.buf, size, > > G_PRIORITY_DEFAULT, > > c->cancellable, demux_to_client_cb, > > client); > > - return; > > } else { > > - demux_to_client_finish(self, client, size); > > + /* Nothing to write */ > > + demux_to_client_finish(client, FALSE); > > } > > } > > > > @@ -377,8 +377,8 @@ static void start_client(SpiceWebdavChannel *self) > > > > g_hash_table_insert(c->clients, &client->id, client); > > > > - client_start_read(self, client); > > - demux_to_client(self, client); > > + client_start_read(client); > > + demux_to_client(client); > > > > g_clear_object(&addr); > > return; > > @@ -417,7 +417,7 @@ static void data_read_cb(GObject *source_object, > > client = g_hash_table_lookup(c->clients, &c->demux.client); > > > > if (client) > > - demux_to_client(self, client); > > + demux_to_client(client); > > else > > start_client(self); > > } > > -- > > 2.4.1 > > > > _______________________________________________ > > Spice-devel mailing list > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > > -- > Marc-André Lureau _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel