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

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,26 +4605,23 @@ 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 {
+        g_free (str);
-        SmsPartContext *ctx;
+        return;
+    }
+    g_free (str);
+
+    /* 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;
+    }
 
     ctx = g_new0 (SmsPartContext, 1);
     ctx->self = g_object_ref (self);
@@ -4637,9 +4634,6 @@ cmti_received (MMAtSerialPort *port,
                                           MM_SMS_STORAGE_UNKNOWN,
                                           (GAsyncReadyCallback)indication_lock_storages_ready,
                                           ctx);
-    }
-
-    g_free (str);
 }
 
 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);
 }
 

src/mm-sms-list.c

@@ -287,6 +287,25 @@ take_multipart (MMSmsList *self,
     return TRUE;
 }
 
+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,
@@ -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 &&
+    if (mm_sms_list_has_part (self,
-        g_list_find_custom (self->priv->list,
+                              storage,
-                            &ctx,
+                              mm_sms_part_get_index (part))) {
-                            (GCompareFunc)cmp_sms_by_part_index_and_storage)) {
         g_set_error (error,
                      MM_CORE_ERROR,
                      MM_CORE_ERROR_FAILED,

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,