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

[Fabiano Fidêncio]

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) {
   ...
}

What do you think about this?

>
> Christophe