[PATCH] core: prefer g_task_return_error_if_cancelled() than custom errors

[Dan Williams]

On Sat, 2017-04-15 at 23:26 +0200, Aleksander Morgado wrote:
> When the GCancellable is added to the GTask, we can use a single
> method call to check for the task being cancelled, and complete it
> right away if so.
> 
> This patch also clears up the logic in the Novatel plugin, where the
> code was trying to return "TRUE" when the task was cancelled, but
> wouldn't work as the check-cancellable flag in the GTask is TRUE by
> default (i.e. when completing the GTask, if it was cancelled, a
> G_IO_ERROR_CANCELLED would be returned by default, regardless of any
> other return value set).
> 
> This patch also introduces a small variation of the logic in the
> Cinterion plugin: instead of running SWWAN=0 before completing the
> async action, the command is now sent just after completion of the
> async action. This shouldn't be an issue, as the SWWAN result itself
> is ignored.
> 

Patch LGTM.

> ---
> 
> Hey Ben,
> 
> What do you think of these changes? I believe we should start using
> this where applicable, instead of the custom checks and errors.
> 
> ---
>  plugins/cinterion/mm-broadband-bearer-cinterion.c | 7 ++-----
>  plugins/novatel/mm-common-novatel.c               | 3 +--
>  src/mm-auth-provider-polkit.c                     | 6 +-----
>  src/mm-iface-modem-signal.c                       | 6 +-----
>  4 files changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/plugins/cinterion/mm-broadband-bearer-cinterion.c
> b/plugins/cinterion/mm-broadband-bearer-cinterion.c
> index 741d2935..a1c699e9 100644
> --- a/plugins/cinterion/mm-broadband-bearer-cinterion.c
> +++ b/plugins/cinterion/mm-broadband-bearer-cinterion.c
> @@ -343,10 +343,6 @@ handle_cancel_dial (GTask *task)
>                                     NULL,
>                                     NULL);
>      g_free (command);
> -
> -    g_task_return_new_error (task, MM_CORE_ERROR,
> MM_CORE_ERROR_CANCELLED,
> -                             "Connection operation has been
> cancelled");
> -    g_object_unref (task);
>  }
> 
>  static void
> @@ -357,8 +353,9 @@ dial_3gpp_context_step (GTask *task)
>      ctx = (Dial3gppContext *) g_task_get_task_data (task);
> 
>      /* Check for cancellation */
> -    if (g_cancellable_is_cancelled (g_task_get_cancellable (task)))
> {
> +    if (g_task_return_error_if_cancelled (task)) {
>          handle_cancel_dial (task);
> +        g_object_unref (task);
>          return;
>      }
> 
> diff --git a/plugins/novatel/mm-common-novatel.c
> b/plugins/novatel/mm-common-novatel.c
> index 96d845f1..f8c78f51 100644
> --- a/plugins/novatel/mm-common-novatel.c
> +++ b/plugins/novatel/mm-common-novatel.c
> @@ -87,10 +87,9 @@ custom_init_step (GTask *task)
>      ctx = g_task_get_task_data (task);
> 
>      /* If cancelled, end */
> -    if (g_cancellable_is_cancelled (g_task_get_cancellable (task)))
> {
> +    if (g_task_return_error_if_cancelled (task)) {
>          mm_dbg ("(Novatel) no need to keep on running custom init in
> (%s)",
>                  mm_port_get_device (MM_PORT (ctx->port)));
> -        g_task_return_boolean (task, TRUE);
>          g_object_unref (task);
>          return;
>      }
> diff --git a/src/mm-auth-provider-polkit.c b/src/mm-auth-provider-
> polkit.c
> index 3b9b4250..7e88d67c 100644
> --- a/src/mm-auth-provider-polkit.c
> +++ b/src/mm-auth-provider-polkit.c
> @@ -72,11 +72,7 @@ check_authorization_ready (PolkitAuthority
> *authority,
>      GError *error = NULL;
>      AuthorizeContext *ctx;
> 
> -    if (g_cancellable_is_cancelled (g_task_get_cancellable (task)))
> {
> -        g_task_return_new_error (task,
> -                                 MM_CORE_ERROR,
> -                                 MM_CORE_ERROR_CANCELLED,
> -                                 "PolicyKit authorization attempt
> cancelled");
> +    if (g_task_return_error_if_cancelled (task)) {
>          g_object_unref (task);
>          return;
>      }
> diff --git a/src/mm-iface-modem-signal.c b/src/mm-iface-modem-
> signal.c
> index 9751b5d6..99096a95 100644
> --- a/src/mm-iface-modem-signal.c
> +++ b/src/mm-iface-modem-signal.c
> @@ -418,11 +418,7 @@ interface_initialization_step (GTask *task)
>      InitializationContext *ctx;
> 
>      /* Don't run new steps if we're cancelled */
> -    if (g_cancellable_is_cancelled (g_task_get_cancellable (task)))
> {
> -        g_task_return_new_error (task,
> -                                 MM_CORE_ERROR,
> -                                 MM_CORE_ERROR_CANCELLED,
> -                                 "Interface initialization
> cancelled");
> +    if (g_task_return_error_if_cancelled (task)) {
>          g_object_unref (task);
>          return;
>      }
> --
> 2.12.2