From 4d43c14e47ae3b67a83ff88bc39c34e02ad15434 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date: Thu, 9 Feb 2012 13:24:57 +0000
Subject: 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>
---
 telepathy-glib/account.c   |  3 ++-
 tests/dbus/account.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 tests/lib/simple-account.c | 32 +++++++++++++++++++++++++++++++-
 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
@@ -252,6 +252,43 @@ test_setters (Test *test,
   tp_clear_object (&test->result);
 }
 
+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,
@@ -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
@@ -74,13 +74,43 @@ struct _TpTestsSimpleAccountPrivate
   gpointer unused;
 };
 
+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
 }
 
-- 
cgit v1.2.1