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

[Victor Toso]

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 at redhat.com> 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 at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
> 
> 
> 
> -- 
> Marc-André Lureau