[Spice-devel] [spice-gtk v3 01/16] file-xfer: get functions for SpiceFileTransferTask

[Victor Toso]

Hi,

On Wed, Jun 01, 2016 at 02:42:58PM +0200, Pavel Grunt wrote:
> Hi Victor,
> 
> On Mon, 2016-05-30 at 11:54 +0200, Victor Toso wrote:
> > In order to channel-main interact with each SpiceFileTransferTask for
> > the file-transfer operation, the following functions are introduced:
> > * spice_file_transfer_task_get_id
> (I would go for spice_file_transfer_task_get_task_id - feel free to ignore it)

Sure!

> > * spice_file_transfer_task_get_channel
> > * spice_file_transfer_task_get_cancellable
> > 
> > Note that although "id" property is public and could be acquired by
> > g_object_get but having the helper function is more practical.
> > 
> > This change is related to split SpiceFileTransferTask from
> > channel-main.
> > ---
> >  src/channel-main.c | 32 +++++++++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 2f29312..89675d5 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -54,6 +54,10 @@
> >  
> >  typedef struct spice_migrate spice_migrate;
> >  
> > +static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
> > +static SpiceMainChannel
> > *spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
> > +static GCancellable
> > *spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
> > +
> >  /**
> >   * SECTION:file-transfer-task
> >   * @short_description: Monitoring file transfers
> > @@ -1902,9 +1906,12 @@ static void file_xfer_data_flushed_cb(GObject
> > *source_object,
> >  static void file_xfer_queue(SpiceFileTransferTask *self, int data_size)
> >  {
> >      VDAgentFileXferDataMessage msg;
> > -    SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(self->channel);
> > +    SpiceMainChannel *channel;
> >  
> > -    msg.id = self->id;
> > +    channel = spice_file_transfer_task_get_channel(self);
> It could be merged with its declaration

Okay

>
> > +    g_return_if_fail(channel != NULL);
> > +
> > +    msg.id = spice_file_transfer_task_get_id(self);
> >      msg.size = data_size;
> >      agent_msg_queue_many(channel, VD_AGENT_FILE_XFER_DATA,
> >                           &msg, sizeof(msg),
> > @@ -1932,12 +1939,15 @@ static void file_xfer_read_cb(GObject *source_object,
> >      }
> >  
> >      if (count > 0 || self->file_size == 0) {
> > +        GCancellable *cancellable;
> > +
> >          self->read_bytes += count;
> >          g_object_notify(G_OBJECT(self), "progress");
> >          file_xfer_queue(self, count);
> >          if (count == 0)
> >              return;
> > -        file_xfer_flush_async(channel, self->cancellable,
> > +        cancellable = spice_file_transfer_task_get_cancellable(self);
> > +        file_xfer_flush_async(channel, cancellable,
> >                                file_xfer_data_flushed_cb, self);
> >          self->pending = TRUE;
> >      } else if (error) {
> > @@ -3221,7 +3231,23 @@ gboolean spice_main_file_copy_finish(SpiceMainChannel
> > *channel,
> >      return g_task_propagate_boolean(task, error);
> >  }
> >  
> > +static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self)
> > +{
> > +    g_return_val_if_fail(self != NULL, 0);
> Not related to your code It makes me wonder if 0 should be considered as invalid
> value. but currently the first transfer task gets 0 as its id.

I thought we started with 1, not 0. I will change the code to start with
1 and making 0 as wrong id.

> > +    return self->id;
> > +}
> > +
> > +static SpiceMainChannel
>
> the return type should be on the same line as the function name - currently we
> are mixing the styles :/

Ouch. Thanks! I'll fix it!

>
> > *spice_file_transfer_task_get_channel(SpiceFileTransferTask *self)
> > +{
> > +    g_return_val_if_fail(self != NULL, NULL);
> > +    return self->channel;
> > +}
> >  
> > +static GCancellable 
> same here

Sure!

Thanks for the review,
  toso

> > *spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self)
> > +{
> > +    g_return_val_if_fail(self != NULL, NULL);
> > +    return self->cancellable;
> > +}
> >  
> >  static void
> >  spice_file_transfer_task_get_property(GObject *object,
> 
> Pavel
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel