Attachment 68273 Details for Bug 55773 – McdConnection: build up our sets of emergency numbers over time

[patch] McdConnection: build up our sets of emergency numbers over time

0002-McdConnection-build-up-our-sets-of-emergency-numbers.patch (text/plain), 6.78 KB, created by Simon McVittie on 2012-10-08 18:46:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2012-10-08 18:46:30 UTC

Size: 6.78 KB

patch

obsolete

>From ac60a0170179b5b405e158bcd0fe6f810ced370c Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Mon, 8 Oct 2012 19:33:15 +0100
>Subject: [PATCH 2/3] McdConnection: build up our sets of emergency numbers
> over time
>
>To facilitate this, use nicer data structures.
>
>Most importantly, this means we don't critical whenever we get more
>than one distinct service point, as Jonny noticed while porting to
>Telepathy 1.0. It might slightly defeat the intention of the
>ServicePointsChanged signal, but in practice the list is probably
>never going to shrink, and if it does, it's probably wise to
>keep considering its old members to be an emergency number (the effect
>that that has is that plugins aren't allowed to influence channel
>requests).
>
>Also, don't leak the sets of emergency numbers when finalized.
>---
> src/mcd-connection-priv.h           |    2 -
> src/mcd-connection-service-points.c |    2 -
> src/mcd-connection.c                |  101 +++++++++++++++--------------------
> 3 files changed, 44 insertions(+), 61 deletions(-)
>
>diff --git a/src/mcd-connection-priv.h b/src/mcd-connection-priv.h
>index 4aefa63..2f73cb9 100644
>--- a/src/mcd-connection-priv.h
>+++ b/src/mcd-connection-priv.h
>@@ -64,8 +64,6 @@ G_GNUC_INTERNAL void _mcd_connection_take_emergency_numbers (McdConnection *self
> G_GNUC_INTERNAL void _mcd_connection_take_emergency_handles (McdConnection *self,
>     TpIntset *handles);
> 
>-G_GNUC_INTERNAL void _mcd_connection_clear_emergency_data (McdConnection *self);
>-
> G_GNUC_INTERNAL gboolean _mcd_connection_target_id_is_urgent (McdConnection *connection,
>     const gchar *name);
> 
>diff --git a/src/mcd-connection-service-points.c b/src/mcd-connection-service-points.c
>index 0e69005..4ab6dc0 100644
>--- a/src/mcd-connection-service-points.c
>+++ b/src/mcd-connection-service-points.c
>@@ -83,8 +83,6 @@ parse_services_list (McdConnection *connection,
>       GSList *service;
>       TpConnection *tp_conn = mcd_connection_get_tp_connection (connection);
> 
>-      _mcd_connection_clear_emergency_data (connection);
>-
>       for (service = e_numbers; service != NULL; service =g_slist_next (service))
>         {
>           if (service->data != NULL)
>diff --git a/src/mcd-connection.c b/src/mcd-connection.c
>index 8792045..b234843 100644
>--- a/src/mcd-connection.c
>+++ b/src/mcd-connection.c
>@@ -135,11 +135,12 @@ struct _McdConnectionPrivate
> 
>     McdSlacker *slacker;
> 
>-    struct
>-    {
>-        TpIntset *handles;
>-        GSList *numbers;
>-    } emergency;
>+    /* Emergency service points' handles.
>+     * Set of handles, lazily-allocated. */
>+    TpIntset *service_point_handles;
>+    /* Emergency service points' identifiers.
>+     * Set of (transfer full) (type utf8), lazily-allocated. */
>+    GHashTable *service_point_ids;
> };
> 
> typedef struct
>@@ -1878,6 +1879,9 @@ _mcd_connection_finalize (GObject * object)
>     if (priv->recognized_presences)
>         g_hash_table_unref (priv->recognized_presences);
> 
>+    tp_clear_pointer (&priv->service_point_handles, tp_intset_destroy);
>+    tp_clear_pointer (&priv->service_point_ids, g_hash_table_unref);
>+
>     G_OBJECT_CLASS (mcd_connection_parent_class)->finalize (object);
> }
> 
>@@ -2143,31 +2147,19 @@ mcd_connection_need_dispatch (McdConnection *connection,
> }
> 
> gboolean
>-_mcd_connection_target_id_is_urgent (McdConnection *self, const gchar *name)
>+_mcd_connection_target_id_is_urgent (McdConnection *self,
>+    const gchar *name)
> {
>-  GSList *list = self->priv->emergency.numbers;
>-
>-  for (; list != NULL; list = g_slist_next (list))
>-    {
>-      const gchar **number = list->data;
>-
>-      for (; number != NULL && *number != NULL; number++)
>-        {
>-          if (!tp_strdiff (*number, name))
>-            return TRUE;
>-        }
>-    }
>-
>-  return FALSE;
>+  return self->priv->service_point_ids != NULL &&
>+      g_hash_table_contains (self->priv->service_point_ids, name);
> }
> 
> gboolean
>-_mcd_connection_target_handle_is_urgent (McdConnection *self, guint handle)
>+_mcd_connection_target_handle_is_urgent (McdConnection *self,
>+    guint handle)
> {
>-  if (handle != 0 && self->priv->emergency.handles != NULL)
>-    return tp_intset_is_member (self->priv->emergency.handles, handle);
>-
>-  return FALSE;
>+  return self->priv->service_point_handles != NULL &&
>+      tp_intset_is_member (self->priv->service_point_handles, handle);
> }
> 
> static gboolean
>@@ -2647,46 +2639,41 @@ _mcd_connection_presence_info_is_ready (McdConnection *self)
>     return self->priv->presence_info_ready;
> }
> 
>-static void clear_emergency_handles (McdConnectionPrivate *priv)
>+void
>+_mcd_connection_take_emergency_numbers (McdConnection *self,
>+    GSList *numbers)
> {
>-  tp_clear_pointer (&priv->emergency.handles, tp_intset_destroy);
>-}
>+  GSList *iter;
> 
>-static void clear_emergency_numbers (McdConnectionPrivate *priv)
>-{
>-  g_slist_foreach (priv->emergency.numbers, (GFunc) g_strfreev, NULL);
>-  tp_clear_pointer (&priv->emergency.numbers, g_slist_free);
>-}
>+  if (self->priv->service_point_ids == NULL)
>+    self->priv->service_point_ids = g_hash_table_new_full (g_str_hash,
>+        g_str_equal, g_free, NULL);
> 
>-void _mcd_connection_clear_emergency_data (McdConnection *self)
>-{
>-    clear_emergency_handles (self->priv);
>-    clear_emergency_numbers (self->priv);
>-}
>-
>-void _mcd_connection_take_emergency_numbers (McdConnection *self, GSList *numbers)
>-{
>-    McdConnectionPrivate *priv = self->priv;
>+  for (iter = numbers; iter != NULL; iter = iter->next)
>+    {
>+      GStrv ids = iter->data;
>+      gchar **id_p;
> 
>-    if (priv->emergency.numbers != NULL)
>-      {
>-        clear_emergency_numbers (priv);
>-        g_critical ("Overwriting old emergency numbers");
>-      }
>+      /* We treat emergency numbers as "sticky": if a given ID has ever
>+       * been considered an emergency number, it stays an emergency number.
>+       * This seems safer than ever removing one and losing their special
>+       * treatment. */
>+      for (id_p = ids; id_p != NULL && *id_p != NULL; id_p++)
>+        g_hash_table_add (self->priv->service_point_ids, *id_p);
>+    }
> 
>-    priv->emergency.numbers = numbers;
>+  /* GStrv members' ownership has already been transferred */
>+  g_slist_free_full (numbers, g_free);
> }
> 
> void
>-_mcd_connection_take_emergency_handles (McdConnection *self, TpIntset *handles)
>+_mcd_connection_take_emergency_handles (McdConnection *self,
>+    TpIntset *handles)
> {
>-    McdConnectionPrivate *priv = self->priv;
>-
>-    if (priv->emergency.handles != NULL)
>-      {
>-        clear_emergency_handles (priv);
>-        g_critical ("Overwriting old emergency handles");
>-      }
>+  if (self->priv->service_point_handles == NULL)
>+    self->priv->service_point_handles = tp_intset_new ();
> 
>-    priv->emergency.handles = handles;
>+  /* As above, we treat emergency numbers as "sticky". */
>+  tp_intset_union_update (self->priv->service_point_handles, handles);
>+  tp_intset_destroy (handles);
> }
>-- 
>1.7.10.4
>

Actions: View | Splinter Review

Attachments on bug 55773: 68272 | 68273 | 68274

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.