<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Apr 17, 2017 at 9:56 AM, Dan Williams <<a href="mailto:dcbw@redhat.com" target="_blank">dcbw@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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. <div class="HOEnZb"><div class="h5"> </div></div></blockquote><div> </div><div>lgtm too</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > --- > > 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 </div></div></blockquote></div> </div></div>