From 815decb03474754a56e7572a2823ce438a81023b Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 12 Sep 2012 10:41:27 +0200 Subject: [PATCH] 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 | 55 +++++++++++++++++----------------------- src/mm-sms-list.c | 26 ++++++++++++++++--- src/mm-sms-list.h | 4 +++ 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 @@ -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 && - 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,