Fwd: [ISSUE] Modem manager SMS storages assert

[Dan Williams]

On Tue, 2013-08-13 at 17:31 +0800, 李晋 wrote:
> Hi,
> 
> This mail is redirected from NM list.

Aleksander, any thoughts here?  I was kinda poking around quickly to
develop the patch I posted, but I wasn't in the code long enough to
really figure out how the storages stuff is supposed to work.

Dan

> Best Regards!
> Quentin Li
> 
> ---------- Forwarded message ----------
> From: 李晋 <snowmanli88 at gmail.com>
> Date: 2013/8/8
> Subject: Re: [ISSUE] Modem manager SMS storages assert
> To: Dan Williams <dcbw at 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 at 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) {
> >
> >
> >
> _______________________________________________
> ModemManager-devel mailing list
> ModemManager-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/modemmanager-devel