tp_account_update_parameters_async: fix lifetime of result, and test it

Without this change to TpAccount, the test would fail with a
use-after-free while inspecting reconnect_required. The TpAccount code
assumed that _finish would always be called directly from the callback,
but it is perfectly valid not to do so.

In the test, also test Reconnect (which is currently unimplemented),
since UpdateParameters and Reconnect are so closely related.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=45554
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Jonny Lamb <jonny.lamb@collabora.co.uk>

3 files changed, 73 insertions, 2 deletions

diff --git a/telepathy-glib/account.c b/telepathy-glib/account.c
index 8eb3aaf65..dc8ca74c8 100644
--- a/telepathy-glib/account.c
+++ b/telepathy-glib/account.c
@@ -2586,7 +2586,8 @@ _tp_account_updated_cb (TpAccount *proxy,
   if (error != NULL)
     g_simple_async_result_set_from_error (result, error);
   else
-    g_simple_async_result_set_op_res_gpointer (result, reconnect_required, NULL);
+    g_simple_async_result_set_op_res_gpointer (result,
+        g_strdupv ((GStrv) reconnect_required), (GDestroyNotify) g_strfreev);
 
   g_simple_async_result_complete (result);
   g_object_unref (G_OBJECT (result));
diff --git a/tests/dbus/account.c b/tests/dbus/account.c
index 79cbc4ffe..2bb4de61e 100644
--- a/tests/dbus/account.c
+++ b/tests/dbus/account.c
@@ -253,6 +253,43 @@ test_setters (Test *test,
 }
 
 static void
+test_reconnect (Test *test,
+    gconstpointer data G_GNUC_UNUSED)
+{
+  GHashTable *set = tp_asv_new (
+      "set", G_TYPE_STRING, "value",
+      NULL);
+  const gchar *unset[] = { "unset", NULL };
+  GStrv reconnect_required;
+
+  test->account = tp_account_new (test->dbus, ACCOUNT_PATH, NULL);
+  g_assert (test->account != NULL);
+
+  tp_account_update_parameters_async (test->account, set, unset,
+      tp_tests_result_ready_cb, &test->result);
+  tp_tests_run_until_result (&test->result);
+  tp_account_update_parameters_finish (test->account, test->result,
+      &reconnect_required, &test->error);
+  g_assert_no_error (test->error);
+  /* check that reconnect_required survives longer than result */
+  tp_clear_object (&test->result);
+
+  g_assert (reconnect_required != NULL);
+  g_assert_cmpstr (reconnect_required[0], ==, "set");
+  g_assert_cmpstr (reconnect_required[1], ==, "unset");
+  g_assert_cmpstr (reconnect_required[2], ==, NULL);
+  g_strfreev (reconnect_required);
+
+  tp_account_reconnect_async (test->account, tp_tests_result_ready_cb,
+      &test->result);
+  tp_tests_run_until_result (&test->result);
+  tp_account_reconnect_finish (test->account, test->result, &test->error);
+  g_assert_error (test->error, TP_ERROR, TP_ERROR_NOT_IMPLEMENTED);
+  g_clear_error (&test->error);
+  tp_clear_object (&test->result);
+}
+
+static void
 account_prepare_cb (GObject *source,
     GAsyncResult *result,
     gpointer user_data)
@@ -778,6 +815,9 @@ main (int argc,
   g_test_add ("/account/setters", Test, NULL, setup_service, test_setters,
       teardown_service);
 
+  g_test_add ("/account/reconnect", Test, NULL, setup_service, test_reconnect,
+      teardown_service);
+
   g_test_add ("/account/prepare/success", Test, NULL, setup_service,
               test_prepare_success, teardown_service);
 
diff --git a/tests/lib/simple-account.c b/tests/lib/simple-account.c
index adc395cc8..bce3c269d 100644
--- a/tests/lib/simple-account.c
+++ b/tests/lib/simple-account.c
@@ -75,12 +75,42 @@ struct _TpTestsSimpleAccountPrivate
 };
 
 static void
+tp_tests_simple_account_update_parameters (TpSvcAccount *svc,
+    GHashTable *parameters,
+    const gchar **unset_parameters,
+    DBusGMethodInvocation *context)
+{
+  GPtrArray *reconnect_required = g_ptr_array_new ();
+  GHashTableIter iter;
+  gpointer k;
+  guint i;
+
+  /* We don't actually store any parameters, but for the purposes
+   * of this method we pretend that every parameter provided is
+   * valid and requires reconnection. */
+
+  g_hash_table_iter_init (&iter, parameters);
+
+  while (g_hash_table_iter_next (&iter, &k, NULL))
+    g_ptr_array_add (reconnect_required, k);
+
+  for (i = 0; unset_parameters != NULL && unset_parameters[i] != NULL; i++)
+    g_ptr_array_add (reconnect_required, (gchar *) unset_parameters[i]);
+
+  g_ptr_array_add (reconnect_required, NULL);
+
+  tp_svc_account_return_from_update_parameters (context,
+      (const gchar **) reconnect_required->pdata);
+  g_ptr_array_unref (reconnect_required);
+}
+
+static void
 account_iface_init (gpointer klass,
     gpointer unused G_GNUC_UNUSED)
 {
 #define IMPLEMENT(x) tp_svc_account_implement_##x (\
   klass, tp_tests_simple_account_##x)
-  /* TODO */
+  IMPLEMENT (update_parameters);
 #undef IMPLEMENT
 }