[Spice-devel] [PATCH v2 04/13] channel-usbredir: Use GTask instead of GSimpleAsyncResult

[Jonathon Jongsma]

ACK with squashed changes below


On Wed, 2016-02-17 at 08:58 +0100, Fabiano Fidêncio wrote:
> On Wed, Feb 17, 2016 at 8:40 AM, Fabiano Fidêncio <fidencio at redhat.com> wrote:
> > On Tue, Feb 16, 2016 at 9:35 PM, Jonathon Jongsma <jjongsma at redhat.com>
> > wrote:
> > > On Fri, 2016-02-12 at 10:46 +0100, Fabiano Fidêncio wrote:
> > > > Instead of using GSimpleAsyncResult, use the new GTask API, which is
> > > > much more straightforward.
> > > > ---
> > > >  src/channel-usbredir.c | 36 ++++++++++++++++--------------------
> > > >  1 file changed, 16 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > > > index a9942b2..2d264f9 100644
> > > > --- a/src/channel-usbredir.c
> > > > +++ b/src/channel-usbredir.c
> > > > @@ -76,7 +76,7 @@ struct _SpiceUsbredirChannelPrivate {
> > > >      int read_buf_size;
> > > >      enum SpiceUsbredirChannelState state;
> > > >  #ifdef USE_POLKIT
> > > > -    GSimpleAsyncResult *result;
> > > > +    GTask *task;
> > > >      SpiceUsbAclHelper *acl_helper;
> > > >  #endif
> > > >  };
> > > > @@ -297,12 +297,14 @@ static void spice_usbredir_channel_open_acl_cb(
> > > >          spice_usbredir_channel_open_device(channel, &err);
> > > >      }
> > > >      if (err) {
> > > > -        g_simple_async_result_take_error(priv->result, err);
> > > > +        g_task_return_error(priv->task, err);
> > > >          libusb_unref_device(priv->device);
> > > >          priv->device = NULL;
> > > >          g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> > > >          priv->spice_device = NULL;
> > > >          priv->state  = STATE_DISCONNECTED;
> > > > +    } else {
> > > > +        g_task_return_boolean(priv->task, TRUE);
> > > >      }
> > > > 
> > > >      spice_usb_acl_helper_close_acl(priv->acl_helper);
> > > > @@ -310,8 +312,7 @@ static void spice_usbredir_channel_open_acl_cb(
> > > >      g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
> > > >                   "inhibit-keyboard-grab", FALSE, NULL);
> > > > 
> > > > -    g_simple_async_result_complete_in_idle(priv->result);
> > > > -    g_clear_object(&priv->result);
> > > > +    g_clear_object(&priv->task);
> > > >  }
> > > >  #endif
> > > > 
> > > > @@ -338,18 +339,17 @@ void spice_usbredir_channel_connect_device_async(
> > > >                    spice_usb_device_get_pid(spice_device),
> > > >                    spice_device, channel);
> > > > 
> > > > -    result = g_simple_async_result_new(G_OBJECT(channel), callback,
> > > > user_data,
> > > > -
> > > >  spice_usbredir_channel_connect_device_async);
> > > > +    task = g_task_new(channel, cancellable, callback, user_data);
> > > 
> > > Are we missing a part of this patch? I would expect to see the 'task'
> > > variable
> > > declared and the 'result' variable declaration removed.
> > 
> > Part of this patch was accidentally merged to
> > cd0c1008316e90bce925e1448ffcabb366e88f8f and then fixed on
> > 8427871a010e19aa1ddb8f853cccae8ab8034540
> 
> And this is what the diff looks like:
> 
> diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> index 537061f..2d264f9 100644
> --- a/src/channel-usbredir.c
> +++ b/src/channel-usbredir.c
> @@ -326,7 +326,7 @@ void spice_usbredir_channel_connect_device_async(
>                                            gpointer              user_data)
>  {
>      SpiceUsbredirChannelPrivate *priv = channel->priv;
> -    GSimpleAsyncResult *result;
> +    GTask *task;
>  #ifndef USE_POLKIT
>      GError *err = NULL;
>  #endif
> 
> 
> > 
> > 
> > > 
> > > 
> > > > 
> > > >      if (!priv->host) {
> > > > -        g_simple_async_result_set_error(result,
> > > > +        g_task_return_new_error(task,
> > > >                              SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > >                              "Error libusb context not set");
> > > >          goto done;
> > > >      }
> > > > 
> > > >      if (priv->state != STATE_DISCONNECTED) {
> > > > -        g_simple_async_result_set_error(result,
> > > > +        g_task_return_new_error(task,
> > > >                              SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > >                              "Error channel is busy");
> > > >          goto done;
> > > > @@ -359,7 +359,7 @@ void spice_usbredir_channel_connect_device_async(
> > > >      priv->spice_device = g_boxed_copy(spice_usb_device_get_type(),
> > > >                                        spice_device);
> > > >  #ifdef USE_POLKIT
> > > > -    priv->result = result;
> > > > +    priv->task = task;
> > > >      priv->state  = STATE_WAITING_FOR_ACL_HELPER;
> > > >      priv->acl_helper = spice_usb_acl_helper_new();
> > > >      g_object_set(spice_channel_get_session(SPICE_CHANNEL(channel)),
> > > > @@ -373,17 +373,18 @@ void spice_usbredir_channel_connect_device_async(
> > > >      return;
> > > >  #else
> > > >      if (!spice_usbredir_channel_open_device(channel, &err)) {
> > > > -        g_simple_async_result_take_error(result, err);
> > > > +        g_task_return_error(task, err);
> > > >          libusb_unref_device(priv->device);
> > > >          priv->device = NULL;
> > > >          g_boxed_free(spice_usb_device_get_type(), priv->spice_device);
> > > >          priv->spice_device = NULL;
> > > > +    } else {
> > > > +        g_task_return_boolean(task, TRUE);
> > > >      }
> > > >  #endif
> > > > 
> > > >  done:
> > > > -    g_simple_async_result_complete_in_idle(result);
> > > > -    g_object_unref(result);
> > > > +    g_object_unref(task);
> > > >  }
> > > > 
> > > >  G_GNUC_INTERNAL
> > > > @@ -392,16 +393,11 @@ gboolean
> > > > spice_usbredir_channel_connect_device_finish(
> > > >                                                 GAsyncResult        
> > > >  *res,
> > > >                                                 GError             
> > > >  **err)
> > > >  {
> > > > -    GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(res);
> > > > +    GTask *task = G_TASK(res);
> > > > 
> > > > -    g_return_val_if_fail(g_simple_async_result_is_valid(res,
> > > > G_OBJECT(channel),
> > > > -
> > > >  spice_usbredir_channel_connect_device_async),
> > > > -                         FALSE);
> > > > +    g_return_val_if_fail(g_task_is_valid(task, channel), FALSE);
> > > > 
> > > > -    if (g_simple_async_result_propagate_error(result, err))
> > > > -        return FALSE;
> > > > -
> > > > -    return TRUE;
> > > > +    return g_task_propagate_boolean(task, err);
> > > >  }
> > > > 
> > > >  G_GNUC_INTERNAL
> > > 
> > > 
> > > Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>