[Spice-devel] [PATCH spice-gtk 13/15] channel-main: Use file_xfer_completed where appropriate

[Hans de Goede]

Hi,

On 03/10/2013 03:35 PM, Marc-André Lureau wrote:
> On Sat, Mar 9, 2013 at 12:06 PM, Hans de Goede <hdegoede at redhat.com> wrote:
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>   gtk/channel-main.c | 37 +++++++++++--------------------------
>>   1 file changed, 11 insertions(+), 26 deletions(-)
>>
>> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
>> index 39061e9..1ae300c 100644
>> --- a/gtk/channel-main.c
>> +++ b/gtk/channel-main.c
>> @@ -164,6 +164,7 @@ static void migrate_channel_event_cb(SpiceChannel *channel, SpiceChannelEvent ev
>>   static gboolean main_migrate_handshake_done(gpointer data);
>>   static void spice_main_channel_send_migration_handshake(SpiceChannel *channel);
>>   static void file_xfer_continue_read(SpiceFileXferTask *task);
>> +static void file_xfer_completed(SpiceFileXferTask *task, GError *error);
>>
>>   /* ------------------------------------------------------------------ */
>>
>> @@ -1589,13 +1590,7 @@ static void file_xfer_data_flushed_cb(GObject *source_object,
>>       task = l->data;
>>
>>       if (error != NULL) {
>> -        g_warning("failed to flush xfer queue: %s", error->message);
>
> In which case the flush can happen? If it's a programming error, it's
> better to keep the warning.

Ok.

>
>> -        task->error = error;
>> -        g_input_stream_close_async(G_INPUT_STREAM(task->file_stream),
>> -                                   G_PRIORITY_DEFAULT,
>> -                                   task->cancellable,
>> -                                   file_xfer_close_cb,
>> -                                   task);
>> +        file_xfer_completed(task, error);
>>           return;
>>       }
>>
>> @@ -1646,12 +1641,7 @@ static void file_xfer_read_cb(GObject *source_object,
>>           agent_msg_queue_many(task->channel, VD_AGENT_FILE_XFER_STATUS,
>>                                &msg, sizeof(msg), NULL);
>>           spice_channel_wakeup(SPICE_CHANNEL(task->channel), FALSE);
>> -        task->error = error;
>> -        g_input_stream_close_async(G_INPUT_STREAM(task->file_stream),
>> -                                   G_PRIORITY_DEFAULT,
>> -                                   task->cancellable,
>> -                                   file_xfer_close_cb,
>> -                                   task);
>> +        file_xfer_completed(task, error);
>>       }
>>       /* else EOF, do nothing (wait for VD_AGENT_FILE_XFER_STATUS from agent) */
>>   }
>> @@ -1675,6 +1665,7 @@ static void file_xfer_handle_status(SpiceMainChannel *channel,
>>       SpiceMainChannelPrivate *c = channel->priv;
>>       GList *l;
>>       SpiceFileXferTask *task;
>> +    GError *error = NULL;
>>
>>       l = g_list_find_custom(c->file_xfer_task_list, &msg->id,
>>                              file_xfer_task_find);
>> @@ -1689,29 +1680,23 @@ static void file_xfer_handle_status(SpiceMainChannel *channel,
>>           file_xfer_continue_read(task);
>>           return;
>>       case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
>> -        SPICE_DEBUG("user removed task %d, result: %d", msg->id,
>> -                    msg->result);
>
> That debug line looked useful, you didn't like it?
>

There already is a debug with similar info before the start of the switch, and on
file_xfer_completed with an error, it will SPICE_DEBUG-log the error, so we are already
logging this twice, 3 times seems a bit overkill...

>> -        task->error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> -                                  "transfer is cancelled by spice agent");
>> +        error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> +                            "transfer is cancelled by spice agent");
>>           break;
>>       case VD_AGENT_FILE_XFER_STATUS_ERROR:
>> -        task->error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> -                                  "some errors occurred in the spice agent");
>> +        error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> +                            "some errors occurred in the spice agent");
>>           break;
>>       case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
>>           break;
>>       default:
>>           g_warn_if_reached();
>> -        task->error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> -                                  "unhandled status type: %u", msg->result);
>> +        error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
>> +                            "unhandled status type: %u", msg->result);
>>           break;
>>       }
>>
>> -    g_input_stream_close_async(G_INPUT_STREAM(task->file_stream),
>> -                               G_PRIORITY_DEFAULT,
>> -                               task->cancellable,
>> -                               file_xfer_close_cb,
>> -                               task);
>> +    file_xfer_completed(task, error);
>>   }
>>
>>   /* coroutine context */
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
>
>

Regards,

Hans