Attachment 56545 Details for Bug 45554 – [5/7] tp_account_update_parameters_async: fix lifetime of result, and test it

[patch] [5/7] tp_account_update_parameters_async: fix lifetime of result, and test it

0005-tp_account_update_parameters_async-fix-lifetime-of-r.patch (text/plain), 4.92 KB, created by Simon McVittie on 2012-02-02 09:29:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2012-02-02 09:29:48 UTC

Size: 4.92 KB

patch

obsolete

>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
>

Actions: View | Splinter Review

Attachments on bug 45554: 56541 | 56542 | 56543 | 56544 | 56545 | 56546 | 56547 | 59796 | 59798

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.