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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> +    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.
> +    return self->id;
> +}
> +
> +static SpiceMainChannel 

the return type should be on the same line as the function name - currently we
are mixing the styles :/

> *spice_file_transfer_task_get_channel(SpiceFileTransferTask *self)
> +{
> +    g_return_val_if_fail(self != NULL, NULL);
> +    return self->channel;
> +}
>  
> +static GCancellable 
same here
> *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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]