[Spice-devel] [PATCH 02/14] channel-main: Use GTask instead of GSimpleAsyncResult

[Christophe Fergeau]

On Wed, Jan 20, 2016 at 02:45:59PM +0100, Fabiano Fidêncio wrote:
> On Wed, Jan 20, 2016 at 2:39 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > I don't really understand what you are getting at.
> > The way I read the code is
> > "if we have an error, then return from the async call and report an
> > error, otherwise keep going or return success", and then
> > g_propagate_boolean in the _finish() method is going to do the right
> > thing for you, report an error if there was one, return a boolean
> > otherwise.
> 
> What I was trying to say is that we have an if-else there, both using
> g_task_return_boolean(task, FALSE/TRUE). Accepting your idea to use
> g_task_return_error() we could change the code for something like
> this:
> 
> ffidenci at cat ~/src/upstream/spice-gtk $ git diff src/channel-main.c
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 6c0f238..52e08ce 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -1770,6 +1770,7 @@ static void file_xfer_close_cb(GObject      *object,
>      GTask *task;
>      SpiceFileTransferTask *self;
>      GError *error = NULL;
> +    gboolean has_error;
> 
>      self = user_data;
> 
> @@ -1789,11 +1790,11 @@ static void file_xfer_close_cb(GObject      *object,
>                        self->priv->cancellable,
>                        self->priv->callback,
>                        self->priv->user_data);
> -    if (self->priv->error) {
> -        g_task_return_error(task, self->priv->error);
> -        g_task_return_boolean(task, FALSE);
> -    } else {
> -        g_task_return_boolean(task, TRUE);
> +
> +    has_error = self->priv->error != NULL;
> +
> +    g_task_return_error(taks, self->priv->error);
> +    if (!has_error) {
>          if (spice_util_get_debug()) {
>              gint64 now = g_get_monotonic_time();
>              gchar *basename = g_file_get_basename(self->priv->file);
> 
> Does it make sense for you?

Not really, the (amended) initial patch is:


     if (self->priv->error) {
-        g_simple_async_result_take_error(res, self->priv->error);
-        g_simple_async_result_set_op_res_gboolean(res, FALSE);
+        g_task_return_error(task, self->priv->error);
     } else {
-        g_simple_async_result_set_op_res_gboolean(res, TRUE);
+        g_task_return_boolean(task, TRUE);
         if (spice_util_get_debug()) {
             gint64 now = g_get_monotonic_time();
             gchar *basename = g_file_get_basename(self->priv->file);
@@ -1819,8 +1810,7 @@ static void file_xfer_close_cb(GObject      *object,
             g_free(transfer_speed_str);
         }
     }
-    g_simple_async_result_complete_in_idle(res);
-    g_object_unref(res);
+    g_object_unref(task);

     g_object_unref(self);
 }

so I'm not sure why you want to add a has_error boolean?

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20160120/bc77066d/attachment.sig>