mcd_account_check_validity, mcd_account_check_parameters: be synchronous

There isn't actually anything in these functions that needs to be async.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71093
Reviewed-by: Guillaume Desmottes <guillaume.desmottes@collabora.co.uk>

3 files changed, 63 insertions, 121 deletions

diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c
index 268205d3..57039253 100644
--- a/src/mcd-account-manager.c
+++ b/src/mcd-account-manager.c
@@ -223,17 +223,6 @@ altered_one_cb (McpAccountStorage *storage,
     }
 }
 
-/* callbacks for the various stages in an backend-driven account creation */
-static void
-async_created_validity_cb (McdAccount *account, const GError *invalid_reason, gpointer data)
-{
-    DEBUG ("asynchronously created account %s is %svalid",
-           mcd_account_get_unique_name (account), (invalid_reason == NULL) ? "" : "in");
-
-    /* safely cached in the accounts hash by now */
-    g_object_unref (account);
-}
-
 static void
 async_created_manager_cb (McdManager *cm, const GError *error, gpointer data)
 {
@@ -242,6 +231,7 @@ async_created_manager_cb (McdManager *cm, const GError *error, gpointer data)
     McdAccountManager *am = lad->account_manager;
     McpAccountStorage *plugin = lad->storage_plugin;
     const gchar *name = NULL;
+    gboolean ok;
 
     g_assert (lad->account_lock > 0);
     g_assert (MCD_IS_ACCOUNT (lad->account));
@@ -264,9 +254,15 @@ async_created_manager_cb (McdManager *cm, const GError *error, gpointer data)
 
     /* this triggers the final parameter check which results in dbus signals *
      * being fired and (potentially) the account going online automatically  */
-    mcd_account_check_validity (account, async_created_validity_cb, NULL);
+    ok = mcd_account_check_validity (account, NULL);
+    DEBUG ("asynchronously created account %s is %svalid",
+           mcd_account_get_unique_name (account),
+           ok ? "" : "in");
 
     g_object_unref (cm);
+
+    /* safely cached in the accounts hash by now */
+    g_object_unref (account);
 }
 
 /* account created by an McpAccountStorage plugin after the initial setup   *
@@ -794,23 +790,6 @@ complete_account_creation_finish (McdAccount *account,
 }
 
 static void
-complete_account_creation_check_validity_cb (McdAccount *account,
-                                             const GError *invalid_reason,
-                                             gpointer user_data)
-{
-    McdCreateAccountData *cad = user_data;
-
-    if (invalid_reason != NULL)
-    {
-        cad->ok = FALSE;
-        g_set_error_literal (&cad->error, invalid_reason->domain,
-            invalid_reason->code, invalid_reason->message);
-    }
-
-    complete_account_creation_finish (account, cad);
-}
-
-static void
 complete_account_creation_set_cb (McdAccount *account, GPtrArray *not_yet,
                                   const GError *set_error, gpointer user_data)
 {
@@ -835,12 +814,12 @@ complete_account_creation_set_cb (McdAccount *account, GPtrArray *not_yet,
     if (cad->ok)
     {
         add_account (account_manager, account, G_STRFUNC);
-        mcd_account_check_validity (account, complete_account_creation_check_validity_cb, cad);
-    }
-    else
-    {
-        complete_account_creation_finish (account, cad);
+
+        if (!mcd_account_check_validity (account, &cad->error))
+            cad->ok = FALSE;
     }
+
+    complete_account_creation_finish (account, cad);
 }
 
 static void
diff --git a/src/mcd-account.c b/src/mcd-account.c
index aea13e25..aa8de240 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -458,44 +458,30 @@ mcd_account_get_parameter_of_known_type (McdAccount *account,
     return FALSE;
 }
 
-typedef void (*CheckParametersCb) (
-    McdAccount *account,
-    const GError *invalid_reason,
-    gpointer user_data);
-static void mcd_account_check_parameters (McdAccount *account,
-    CheckParametersCb callback, gpointer user_data);
-
-static void
-manager_ready_check_params_cb (McdAccount *account,
-    const GError *invalid_reason,
-    gpointer user_data)
-{
-    McdAccountPrivate *priv = account->priv;
-
-    g_clear_error (&priv->invalid_reason);
-    if (invalid_reason != NULL)
-    {
-        priv->invalid_reason = g_error_copy (invalid_reason);
-    }
-
-    mcd_account_loaded (account);
-}
+static gboolean mcd_account_check_parameters (McdAccount *account,
+    GError **invalid_reason);
 
 static void on_manager_ready (McdManager *manager, const GError *error,
                               gpointer user_data)
 {
     McdAccount *account = MCD_ACCOUNT (user_data);
+    GError *invalid_reason = NULL;
 
     if (error)
     {
         DEBUG ("got error: %s", error->message);
-        mcd_account_loaded (account);
+    }
+    else if (!mcd_account_check_parameters (account, &invalid_reason))
+    {
+        g_clear_error (&account->priv->invalid_reason);
+        account->priv->invalid_reason = invalid_reason;
     }
     else
     {
-        mcd_account_check_parameters (account, manager_ready_check_params_cb,
-                                      NULL);
+        g_clear_error (&account->priv->invalid_reason);
     }
+
+    mcd_account_loaded (account);
 }
 
 static gboolean
@@ -2280,25 +2266,22 @@ mcd_account_altered_by_plugin (McdAccount *account,
 }
 
 
-static void
+static gboolean
 mcd_account_check_parameters (McdAccount *account,
-                              CheckParametersCb callback,
-                              gpointer user_data)
+                              GError **error)
 {
     McdAccountPrivate *priv = account->priv;
     TpProtocol *protocol;
     GList *params = NULL;
     GList *iter;
-    GError *error = NULL;
-
-    g_return_if_fail (callback != NULL);
+    GError *inner_error = NULL;
 
     DEBUG ("called for %s", priv->unique_name);
     protocol = _mcd_manager_dup_protocol (priv->manager, priv->protocol_name);
 
     if (protocol == NULL)
     {
-        g_set_error (&error, TP_ERROR, TP_ERROR_INVALID_ARGUMENT,
+        g_set_error (&inner_error, TP_ERROR, TP_ERROR_INVALID_ARGUMENT,
             "CM '%s' doesn't implement protocol '%s'", priv->manager_name,
             priv->protocol_name);
         goto out;
@@ -2315,7 +2298,7 @@ mcd_account_check_parameters (McdAccount *account,
 
         if (!mcd_account_get_parameter (account, param, NULL, NULL))
         {
-            g_set_error (&error, TP_ERROR, TP_ERROR_INVALID_ARGUMENT,
+            g_set_error (&inner_error, TP_ERROR, TP_ERROR_INVALID_ARGUMENT,
                 "missing required parameter '%s'",
                 tp_connection_manager_param_get_name (param));
             goto out;
@@ -2323,26 +2306,22 @@ mcd_account_check_parameters (McdAccount *account,
     }
 
 out:
-    if (error != NULL)
+    if (inner_error != NULL)
     {
-        DEBUG ("%s", error->message);
+        DEBUG ("%s", inner_error->message);
     }
 
-    callback (account, error, user_data);
-    g_clear_error (&error);
     g_list_free_full (params,
                       (GDestroyNotify) tp_connection_manager_param_free);
     g_clear_object (&protocol);
-}
 
-static void
-set_parameters_maybe_autoconnect_cb (McdAccount *account,
-                                     const GError *invalid_reason,
-                                     gpointer user_data G_GNUC_UNUSED)
-{
-    /* Strictly speaking this doesn't need to be called unless invalid_reason
-     * is NULL, but calling it in all cases gives us clearer debug output */
-    _mcd_account_maybe_autoconnect (account);
+    if (inner_error != NULL)
+    {
+        g_propagate_error (error, inner_error);
+        return FALSE;
+    }
+
+    return TRUE;
 }
 
 static void
@@ -2380,8 +2359,11 @@ apply_parameter_updates (McdAccount *account,
         }
     }
 
-    mcd_account_check_validity (account,
-                                set_parameters_maybe_autoconnect_cb, NULL);
+    mcd_account_check_validity (account, NULL);
+
+    /* Strictly speaking this doesn't need to be called if not valid,
+     * but calling it in all cases gives us clearer debug output */
+    _mcd_account_maybe_autoconnect (account);
 }
 
 static void
@@ -4327,23 +4309,22 @@ mcd_account_get_connection (McdAccount *account)
     return priv->connection;
 }
 
-typedef struct
-{
-    McdAccountCheckValidityCb callback;
-    gpointer user_data;
-} CheckValidityData;
-
-static void
-check_validity_check_parameters_cb (McdAccount *account,
-                                    const GError *invalid_reason,
-                                    gpointer user_data)
+gboolean
+mcd_account_check_validity (McdAccount *account,
+    GError **error)
 {
-    CheckValidityData *data = (CheckValidityData *) user_data;
     McdAccountPrivate *priv = account->priv;
-    gboolean now_valid = (invalid_reason == NULL);
-    gboolean was_valid = (priv->invalid_reason == NULL);
+    GError *invalid_reason = NULL;
+    gboolean now_valid;
+    gboolean was_valid;
+
+    g_return_val_if_fail (MCD_IS_ACCOUNT (account), FALSE);
+
+    was_valid = (priv->invalid_reason == NULL);
+    now_valid = mcd_account_check_parameters (account, &invalid_reason);
 
     g_clear_error (&priv->invalid_reason);
+
     if (invalid_reason != NULL)
     {
         priv->invalid_reason = g_error_copy (invalid_reason);
@@ -4369,27 +4350,13 @@ check_validity_check_parameters_cb (McdAccount *account,
         }
     }
 
-    if (data->callback != NULL)
-        data->callback (account, invalid_reason, data->user_data);
-
-    g_slice_free (CheckValidityData, data);
-}
-
-void
-mcd_account_check_validity (McdAccount *account,
-                            McdAccountCheckValidityCb callback,
-                            gpointer user_data)
-{
-    CheckValidityData *data;
-
-    g_return_if_fail (MCD_IS_ACCOUNT (account));
-
-    data = g_slice_new0 (CheckValidityData);
-    data->callback = callback;
-    data->user_data = user_data;
+    if (invalid_reason != NULL)
+    {
+        g_propagate_error (error, invalid_reason);
+        return FALSE;
+    }
 
-    mcd_account_check_parameters (account, check_validity_check_parameters_cb,
-                                  data);
+    return TRUE;
 }
 
 /*
diff --git a/src/mcd-account.h b/src/mcd-account.h
index 86d2b82b..85a56cce 100644
--- a/src/mcd-account.h
+++ b/src/mcd-account.h
@@ -96,12 +96,8 @@ const gchar *mcd_account_get_object_path (McdAccount *account);
 
 gboolean mcd_account_is_valid (McdAccount *account);
 
-typedef void (*McdAccountCheckValidityCb) (McdAccount *account,
-                                           const GError *invalid_reason,
-                                           gpointer user_data);
-void mcd_account_check_validity (McdAccount *account,
-                                 McdAccountCheckValidityCb callback,
-                                 gpointer user_data);
+gboolean mcd_account_check_validity (McdAccount *account,
+    GError **error);
 
 gboolean mcd_account_is_enabled (McdAccount *account);