telepathy-mission-control: McdConnection: build up our sets of emergency numbers over time

[Simon McVittie]

Module: telepathy-mission-control
Branch: master
Commit: 8198d1ec4a318338d02800cf6d021734e7590daa
URL:    http://cgit.freedesktop.org/telepathy/telepathy-mission-control/commit/?id=8198d1ec4a318338d02800cf6d021734e7590daa

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Mon Oct  8 19:33:15 2012 +0100

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.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=55773
Reviewed-by: Marco Barisione <marco.barisione at collabora.co.uk>

---

 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 8f03f16..c3866c3 100644
--- a/src/mcd-connection-priv.h
+++ b/src/mcd-connection-priv.h
@@ -61,8 +61,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 9e3ef29..0d231df 100644
--- a/src/mcd-connection.c
+++ b/src/mcd-connection.c
@@ -128,11 +128,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
@@ -1547,6 +1548,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);
 }
 
@@ -1824,31 +1828,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
@@ -2336,46 +2328,41 @@ _mcd_connection_presence_info_is_ready (McdConnection *self)
     return self->priv->presence_info_ready;
 }
 
-static void clear_emergency_handles (McdConnectionPrivate *priv)
-{
-  tp_clear_pointer (&priv->emergency.handles, tp_intset_destroy);
-}
-
-static void clear_emergency_numbers (McdConnectionPrivate *priv)
+void
+_mcd_connection_take_emergency_numbers (McdConnection *self,
+    GSList *numbers)
 {
-  g_slist_foreach (priv->emergency.numbers, (GFunc) g_strfreev, NULL);
-  tp_clear_pointer (&priv->emergency.numbers, g_slist_free);
-}
+  GSList *iter;
 
-void _mcd_connection_clear_emergency_data (McdConnection *self)
-{
-    clear_emergency_handles (self->priv);
-    clear_emergency_numbers (self->priv);
-}
+  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_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);
 }