broadband-modem: properly avoid duplicate CMTI indications

We will look for the part being notified in the CMTI indication directly in the
list of processed parts; and if we have it there already we won't re-process it.

Still, we may get several CMTI indications for the same part one after the
other, so it may still happen that the second CMTI comes *before* we have parsed
and created the part in the SMS list. For that case, the SMS list will reject
taking the part if there is already a part with the same storage+index already
processed.

This fix removes the `known_sms_parts' hash table, which was being handled
incorrectly.

This should fix https://bugzilla.gnome.org/show_bug.cgi?id=675175

3 files changed, 49 insertions, 36 deletions

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index b51a5c9f..a0c0651d 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -165,7 +165,6 @@ struct _MMBroadbandModemPrivate {
     MMSmsStorage modem_messaging_sms_default_storage;
     /* Implementation helpers */
     gboolean sms_supported_modes_checked;
-    GHashTable *known_sms_parts;
     gboolean mem1_storage_locked;
     MMSmsStorage current_sms_mem1_storage;
     gboolean mem2_storage_locked;
@@ -4598,6 +4597,7 @@ cmti_received (MMAtSerialPort *port,
                GMatchInfo *info,
                MMBroadbandModem *self)
 {
+    SmsPartContext *ctx;
     guint idx = 0;
     MMSmsStorage storage;
     gchar *str;
@@ -4605,41 +4605,35 @@ cmti_received (MMAtSerialPort *port,
     if (!mm_get_uint_from_match_info (info, 2, &idx))
         return;
 
-    if (G_UNLIKELY (!self->priv->known_sms_parts))
-        self->priv->known_sms_parts = g_hash_table_new (g_direct_hash, g_direct_equal);
-    else if (g_hash_table_lookup_extended (self->priv->known_sms_parts,
-                                           GUINT_TO_POINTER (idx),
-                                           NULL,
-                                           NULL))
-        /* Don't signal multiple times if there are multiple CMTI notifications for a message */
-        return;
-
-    /* Nothing is currently stored in the hash table - presence is all that matters. */
-    g_hash_table_insert (self->priv->known_sms_parts, GUINT_TO_POINTER (idx), NULL);
-
     /* The match info gives us in which storage the index applies */
     str = mm_get_string_unquoted_from_match_info (info, 1);
     storage = mm_common_get_sms_storage_from_string (str, NULL);
-
-    if (storage == MM_SMS_STORAGE_UNKNOWN)
+    if (storage == MM_SMS_STORAGE_UNKNOWN) {
         mm_dbg ("Skipping CMTI indication, unknown storage '%s' reported", str);
-    else {
-        SmsPartContext *ctx;
+        g_free (str);
+        return;
+    }
+    g_free (str);
 
-        ctx = g_new0 (SmsPartContext, 1);
-        ctx->self = g_object_ref (self);
-        ctx->result = g_simple_async_result_new (G_OBJECT (self), NULL, NULL, cmti_received);
-        ctx->idx = idx;
-
-        /* First, request to set the proper storage to read from */
-        mm_broadband_modem_lock_sms_storages (ctx->self,
-                                              storage,
-                                              MM_SMS_STORAGE_UNKNOWN,
-                                              (GAsyncReadyCallback)indication_lock_storages_ready,
-                                              ctx);
+    /* Don't signal multiple times if there are multiple CMTI notifications for a message */
+    if (mm_sms_list_has_part (self->priv->modem_messaging_sms_list,
+                              storage,
+                              idx)) {
+        mm_dbg ("Skipping CMTI indication, part already processed");
+        return;
     }
 
-    g_free (str);
+    ctx = g_new0 (SmsPartContext, 1);
+    ctx->self = g_object_ref (self);
+    ctx->result = g_simple_async_result_new (G_OBJECT (self), NULL, NULL, cmti_received);
+    ctx->idx = idx;
+
+    /* First, request to set the proper storage to read from */
+    mm_broadband_modem_lock_sms_storages (ctx->self,
+                                          storage,
+                                          MM_SMS_STORAGE_UNKNOWN,
+                                          (GAsyncReadyCallback)indication_lock_storages_ready,
+                                          ctx);
 }
 
 static void
@@ -8071,9 +8065,6 @@ finalize (GObject *object)
     if (self->priv->modem_3gpp_registration_regex)
         mm_3gpp_creg_regex_destroy (self->priv->modem_3gpp_registration_regex);
 
-    if (self->priv->known_sms_parts)
-        g_hash_table_unref (self->priv->known_sms_parts);
-
     G_OBJECT_CLASS (mm_broadband_modem_parent_class)->finalize (object);
 }
 
diff --git a/src/mm-sms-list.c b/src/mm-sms-list.c
index 5bb0e924..9afe96f6 100644
--- a/src/mm-sms-list.c
+++ b/src/mm-sms-list.c
@@ -288,6 +288,25 @@ take_multipart (MMSmsList *self,
 }
 
 gboolean
+mm_sms_list_has_part (MMSmsList *self,
+                      MMSmsStorage storage,
+                      guint index)
+{
+    PartIndexAndStorage ctx;
+
+    if (storage == MM_SMS_STORAGE_UNKNOWN ||
+        index == SMS_PART_INVALID_INDEX)
+        return FALSE;
+
+    ctx.part_index = index;
+    ctx.storage = storage;
+
+    return !!g_list_find_custom (self->priv->list,
+                                 &ctx,
+                                 (GCompareFunc)cmp_sms_by_part_index_and_storage);
+}
+
+gboolean
 mm_sms_list_take_part (MMSmsList *self,
                        MMSmsPart *part,
                        MMSmsState state,
@@ -300,10 +319,9 @@ mm_sms_list_take_part (MMSmsList *self,
     ctx.storage = storage;
 
     /* Ensure we don't have already taken a part with the same index */
-    if (mm_sms_part_get_index (part) != SMS_PART_INVALID_INDEX &&
-        g_list_find_custom (self->priv->list,
-                            &ctx,
-                            (GCompareFunc)cmp_sms_by_part_index_and_storage)) {
+    if (mm_sms_list_has_part (self,
+                              storage,
+                              mm_sms_part_get_index (part))) {
         g_set_error (error,
                      MM_CORE_ERROR,
                      MM_CORE_ERROR_FAILED,
diff --git a/src/mm-sms-list.h b/src/mm-sms-list.h
index f7710227..16f39e51 100644
--- a/src/mm-sms-list.h
+++ b/src/mm-sms-list.h
@@ -61,6 +61,10 @@ MMSmsList *mm_sms_list_new (MMBaseModem *modem);
 GStrv mm_sms_list_get_paths (MMSmsList *self);
 guint mm_sms_list_get_count (MMSmsList *self);
 
+gboolean mm_sms_list_has_part (MMSmsList *self,
+                               MMSmsStorage storage,
+                               guint index);
+
 gboolean mm_sms_list_take_part (MMSmsList *self,
                                 MMSmsPart *part,
                                 MMSmsState state,