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@collabora.co.uk>

3 files changed, 44 insertions, 61 deletions

diff --git a/src/mcd-connection-priv.h b/src/mcd-connection-priv.h
index 8f03f16a..c3866c32 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 0e690056..4ab6dc07 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 9e3ef298..0d231df7 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);
 }