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

[Christophe Fergeau]

On Wed, Jan 20, 2016 at 02:29:19PM +0100, Fabiano Fidêncio wrote:
> On Wed, Jan 20, 2016 at 2:03 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > On Wed, Jan 20, 2016 at 01:38:52PM +0100, Fabiano Fidêncio wrote:
> >> On Mon, Jan 18, 2016 at 11:31 AM, Christophe Fergeau
> >> <cfergeau at redhat.com> wrote:
> >> > On Mon, Jan 18, 2016 at 10:05:38AM +0100, Fabiano Fidêncio wrote:
> >> >> @@ -1794,15 +1785,15 @@ static void file_xfer_close_cb(GObject      *object,
> >> >>
> >> >>      /* Notify to user that files have been transferred or something error
> >> >>         happened. */
> >> >> -    res = g_simple_async_result_new(G_OBJECT(self->priv->channel),
> >> >> -                                    self->priv->callback,
> >> >> -                                    self->priv->user_data,
> >> >> -                                    spice_main_file_copy_async);
> >> >> +    task = g_task_new(self->priv->channel,
> >> >> +                      self->priv->cancellable,
> >> >> +                      self->priv->callback,
> >> >> +                      self->priv->user_data);
> >> >>      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);
> >> >> +        g_task_return_boolean(task, FALSE);
> >> >
> >> > Not sure what GTask behaviour will be if you queue these 2 calls. Just
> >> > calling g_task_return_error() is going to be enough as
> >> > g_task_propagate_boolean() returns FALSE when an error is set.
> >>
> >> Hmm. It shouldn't be in this way. I missed this part :-\
> >> So, I do believe that in case of explicit errors we can call
> >> g_task_return_boolean() instead of g_task_return_error() (as I did
> >> with the other situations similar to this one).
> >>
> >> Is okay for you if I do the same here?
> >
> > I would do
> >       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);
> >
> >
> > it seems you are suggesting keeping the other one?
> > (g_task_return_boolean).
> 
> 
> Yep. At this point you already know that an error has occurred. That's
> the reason I would prefer to have a g_task_return_boolean().
> 
> Otherwise we could have something like:
> 
> gboolean has_error = self->priv->error != NULL;
> g_task_return_error(task, self->priv->error)
> if (!has_error) {
>    ...
> }

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.

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/d4406937/attachment-0001.sig>