From 2fb3097f1d6c80854911111d7857db543587f001 Mon Sep 17 00:00:00 2001
From: Simon McVittie <simon.mcvittie@collabora.co.uk>
Date: Thu, 2 Feb 2012 13:39:08 +0000
Subject: [PATCH 5/7] 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.
---
 telepathy-glib/account.c   |    3 ++-
 tests/dbus/account.c       |   41 +++++++++++++++++++++++++++++++++++++++++
 tests/lib/simple-account.c |   32 +++++++++++++++++++++++++++++++-
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/telepathy-glib/account.c b/telepathy-glib/account.c
index 8eb3aaf..dc8ca74 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 79cbc4f..147b4fb 100644
--- a/tests/dbus/account.c
+++ b/tests/dbus/account.c
@@ -253,6 +253,44 @@ 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,
+      "/org/freedesktop/Telepathy/Account/what/ev/er", 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 +816,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 adc395c..bce3c26 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
 }
 
-- 
1.7.9