Fwd: [ISSUE] Modem manager SMS storages assert

[李晋 <snowmanli88 gmail com>]

Hi,

Patch below can fix message storage assert for UNKNOWN response of "+CMPS?".

Best Regards!

Quentin Li

---------- Forwarded message ----------
From: 李晋 <snowmanli88 gmail com>
Date: 2013/8/8
Subject: Re: [ISSUE] Modem manager SMS storages assert
To: Dan Williams <dcbw redhat com>

Dear Dan,

Because there may be more than 1 UNKNOWN SMS storages in the array, 

so just once skipping still got the same assert. 

I have tried patch below and it works well until now.

--- /home/user/桌面/ModemManager-1.0.0/src/mm-iface-modem-messaging.c 2013-07-19 16:00:01.000000000 +0800

+++ /home/user/ModemManager-1.0.0/src/mm-iface-modem-messaging.c 2013-08-08 16:10:30.004191341 +0800

@@ -795,13 +795,29 @@

 

     if (!storage_ctx->supported_mem1 || ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)

         all_loaded = TRUE;

-    /* We'll skip the 'MT' storage, as that is a combination of 'SM' and 'ME' */

-    else if (g_array_index (storage_ctx->supported_mem1,

-                            MMSmsStorage,

-                            ctx->mem1_storage_index) == MM_SMS_STORAGE_MT) {

-        ctx->mem1_storage_index++;

-        if (ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)

-            all_loaded = TRUE;

+    /* We'll skip the 'MT' storage, as that is a combination of 'SM' and

+     * 'ME'. Also skip unknown storages since they can't be handled.

+     */

+    else 

+    {

+        MMSmsStorage s;

+        

+        while(1)

+        {

+            s = g_array_index (storage_ctx->supported_mem1,

+                                MMSmsStorage,

+                                ctx->mem1_storage_index);

+            if(s == MM_SMS_STORAGE_MT || s == MM_SMS_STORAGE_UNKNOWN) {

+                ctx->mem1_storage_index++;

+                if (ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)

+                {    

+                    all_loaded = TRUE;

+                    break;

+                }

+            }

+            else

+                break;

+        }

     }

 

     if (all_loaded) {

2013/8/8 Dan Williams <dcbw redhat com>

On Wed, 2013-08-07 at 16:44 +0800, 李晋 wrote:
> Hi,
> When I use MediaTek data card device with MM v1.0.0, after I send command
> "mmcli -m 0 -e", MM stopped and assert happened.
> The assert is in function "mm_broadband_modem_lock_sms_storages" in
> mm-broadband-modem.c.
> When I trace the log, I found the root cause:
> Our device response "AT+CMPS?" with some words like "SM_P" and "ME_P",
> these words are parsed by mm_3gpp_parse_cpms_test_response and recognized
> as "MM_SMS_STORAGE_UNKNOWN". So in this case, there were two "UNKNOWN"
> members in storage_ctx->supported_mem1. When modem enabling operation go to
> function "load_initial_sms_parts_from_storages", it try to do
> load_initial_sms_parts but the second param is "MM_SMS_STORAGE_UNKNOWN",
> assertion happen.
> I think maybe for these UNKNOWN storage, load_initial_sms_parts shouldn't
> try to read SMS from it. Instead,  just do as "MM_SMS_STORAGE_MT" in
> function "load_initial_sms_parts_from_storages", skipping them and just try
> to read known storage.

Does this patch work?

Dan

diff --git a/src/mm-iface-modem-messaging.c b/src/mm-iface-modem-messaging.c
index 143d7ef..97ab8fb 100644
--- a/src/mm-iface-modem-messaging.c
+++ b/src/mm-iface-modem-messaging.c
@@ -795,13 +795,20 @@ load_initial_sms_parts_from_storages (EnablingContext *ctx)

     if (!storage_ctx->supported_mem1 || ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)
         all_loaded = TRUE;
-    /* We'll skip the 'MT' storage, as that is a combination of 'SM' and 'ME' */
-    else if (g_array_index (storage_ctx->supported_mem1,
-                            MMSmsStorage,
-                            ctx->mem1_storage_index) == MM_SMS_STORAGE_MT) {
-        ctx->mem1_storage_index++;
-        if (ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)
-            all_loaded = TRUE;
+    else {
+        MMSmsStorage s;
+
+        s = g_array_index (storage_ctx->supported_mem1,
+                           MMSmsStorage,
+                           ctx->mem1_storage_index);
+        /* We'll skip the 'MT' storage, as that is a combination of 'SM' and
+         * 'ME'.  Also skip unknown storages since they can't be handled.
+         */
+        if (s == MM_SMS_STORAGE_MT || s == MM_SMS_STORAGE_UNKNOWN) {
+            ctx->mem1_storage_index++;
+            if (ctx->mem1_storage_index >= storage_ctx->supported_mem1->len)
+                all_loaded = TRUE;
+        }
     }

     if (all_loaded) {