[Spice-devel] [spice-gtk PATCH v1 2/3] webdav: small cleanups

[Marc-André Lureau]

Hi

On Mon, May 18, 2015 at 9:09 AM, Victor Toso <victortoso at redhat.com> wrote:

> changes:
> - unecessary return on demux_to_client;
>

that doesn't hurt, does it? it makes things more explicit perhaps


> - 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 :)


Please split the commit for easier review


> ---
>  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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>



-- 
Marc-André Lureau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150518/30535f55/attachment-0001.html>