On Thu, 2016-06-23 at 19:37 +0200, Victor Toso wrote:
> ---
> src/channel-main.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 63f0b9b..2c9a6fb 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -76,6 +76,7 @@ static gssize
> spice_file_transfer_task_read_finish(SpiceFileTransferTask *self,
> GError **error);
> static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask
> *self);
> static guint64 spice_file_transfer_task_get_bytes_read(SpiceFileTransferTask
> *self);
> +static void spice_file_transfer_task_debug_info(SpiceFileTransferTask *self);
>
> /**
> * SECTION:file-transfer-task
> @@ -1893,18 +1894,8 @@ static void file_xfer_data_flushed_cb(GObject
> *source_object,
>
> file_transfer_operation_send_progress(self);
>
> - if (spice_util_get_debug()) {
> - const GTimeSpan interval = 20 * G_TIME_SPAN_SECOND;
> - gint64 now = g_get_monotonic_time();
> -
> - if (interval < now - self->last_update) {
> - gchar *basename = g_file_get_basename(self->file);
> - self->last_update = now;
> - SPICE_DEBUG("transferred %.2f%% of the file %s",
> - 100.0 * self->read_bytes / self->file_size,
> basename);
> - g_free(basename);
> - }
> - }
> + if (spice_util_get_debug())
> + spice_file_transfer_task_debug_info(self);
>
> /* Read more data */
> spice_file_transfer_task_read_async(self, file_xfer_read_async_cb, NULL);
> @@ -3532,6 +3523,20 @@ static gssize
> spice_file_transfer_task_read_finish(SpiceFileTransferTask *self,
> return nbytes;
> }
>
> +static void spice_file_transfer_task_debug_info(SpiceFileTransferTask *self)
> +{
> + const GTimeSpan interval = 20 * G_TIME_SPAN_SECOND;
> + gint64 now = g_get_monotonic_time();
> +
> + if (interval < now - self->last_update) {
> + gchar *basename = g_file_get_basename(self->file);
> + self->last_update = now;
> + SPICE_DEBUG("transferred %.2f%% of the file %s",
> + 100.0 * self->read_bytes / self->file_size, basename);
> + g_free(basename);
> + }
> +}
It seems a bit odd to embed the time-based reporting policy inside of this
function. I understand that you're extracting this so that it can later be moved
to a separate file and the task will remain encapsulated. So maybe that's fine.
Or there are a couple alternatives we could consider:
- print a debug progress statement after reading the file instead of after
flushing it -- then we could keep the entire thing inside of the spice-file-
transfer-task.c and wouldn't need to expose a _debug_info() function to be
called from channel-main.c
- print a debug progress statement at the FileTransferOperation level rather
than at the Task level. Then we could move keep it all contained within channel-
main.c
Again, I may be being too nitpicky here, so feel free to ignore if you want.
> +
> static void
> spice_file_transfer_task_get_property(GObject *object,
> guint property_id,
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel