broadband-modem: update CMGL parsing logic

Pantech UMW190 modem uses a custom +CMGL response which includes only
three fields before the actual PDU, e.g:

  +CMGL: <index>,<status>,<something>\r\n<PDU>

instead of what we had before:

  +CMGL: <index>,<status>,<alpha>,<length>\r\n<PDU>

The CMGL parsing logic is now updated to use a regex to match the reply, and
also considering the UMW190 specific case.

Actually, we end up reading only the two first fields (index and status) which
are the ones we really need, so we skip the <length> and the <alpha> if given.

Added also unit tests to cover all these known cases.

Partially fixes https://bugzilla.gnome.org/show_bug.cgi?id=696723 (missing the
actual PDU parsing fixes).

4 files changed, 268 insertions, 30 deletions

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 8ec4d1e6..de09d1cb 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -5748,6 +5748,8 @@ sms_pdu_part_list_ready (MMBroadbandModem *self,
 {
     const gchar *response;
     GError *error = NULL;
+    GList *info_list;
+    GList *l;
 
     /* Always always always unlock mem1 storage. Warned you've been. */
     mm_broadband_modem_unlock_sms_storages (self, TRUE, FALSE);
@@ -5759,46 +5761,33 @@ sms_pdu_part_list_ready (MMBroadbandModem *self,
         return;
     }
 
-    while (*response) {
-        MMSmsPart *part;
-        gint idx;
-        gint status;
-        gint tpdu_len;
-        gchar pdu[SMS_MAX_PDU_LEN + 1];
-        gint offset;
-        gint rv;
-
-        rv = sscanf (response,
-                     "+CMGL: %d,%d,,%d %" G_STRINGIFY (SMS_MAX_PDU_LEN) "s %n",
-                     &idx, &status, &tpdu_len, pdu, &offset);
-        if (4 != rv) {
-            g_simple_async_result_set_error (ctx->result,
-                                             MM_CORE_ERROR,
-                                             MM_CORE_ERROR_INVALID_ARGS,
-                                             "Couldn't parse SMS list response: "
-                                             "only %d fields parsed",
-                                             rv);
-            list_parts_context_complete_and_free (ctx);
-            return;
-        }
+    info_list = mm_3gpp_parse_pdu_cmgl_response (response, &error);
+    if (error) {
+        g_simple_async_result_take_error (ctx->result, error);
+        list_parts_context_complete_and_free (ctx);
+        return;
+    }
 
-        /* Will try to keep on the loop */
-        response += offset;
+    for (l = info_list; l; l = g_list_next (l)) {
+        MM3gppPduInfo *info = l->data;
+        MMSmsPart *part;
 
-        part = mm_sms_part_new_from_pdu (idx, pdu, &error);
+        part = mm_sms_part_new_from_pdu (info->index, info->pdu, &error);
         if (part) {
-            mm_dbg ("Correctly parsed PDU (%d)", idx);
+            mm_dbg ("Correctly parsed PDU (%d)", info->index);
             mm_iface_modem_messaging_take_part (MM_IFACE_MODEM_MESSAGING (self),
                                                 part,
-                                                sms_state_from_index (status),
+                                                sms_state_from_index (info->status),
                                                 ctx->list_storage);
         } else {
             /* Don't treat the error as critical */
-            mm_dbg ("Error parsing PDU (%d): %s", idx, error->message);
+            mm_dbg ("Error parsing PDU (%d): %s", info->index, error->message);
             g_clear_error (&error);
         }
     }
 
+    mm_3gpp_pdu_info_list_free (info_list);
+
     /* We consider all done */
     g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE);
     list_parts_context_complete_and_free (ctx);
diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index b20ff1c6..9652c6f4 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -28,6 +28,7 @@
 #define _LIBMM_INSIDE_MM
 #include <libmm-glib.h>
 
+#include "mm-sms-part.h"
 #include "mm-modem-helpers.h"
 #include "mm-log.h"
 
@@ -1433,6 +1434,72 @@ done:
 
 /*************************************************************************/
 
+static void
+mm_3gpp_pdu_info_free (MM3gppPduInfo *info)
+{
+    g_free (info->pdu);
+    g_free (info);
+}
+
+void
+mm_3gpp_pdu_info_list_free (GList *info_list)
+{
+    g_list_free_full (info_list, (GDestroyNotify)mm_3gpp_pdu_info_free);
+}
+
+GList *
+mm_3gpp_parse_pdu_cmgl_response (const gchar *str,
+                                 GError **error)
+{
+    GError *inner_error = NULL;
+    GList *list = NULL;
+    GMatchInfo *match_info;
+    GRegex *r;
+
+    /*
+     * +CMGL: <index>, <status>, [<alpha>], <length>
+     *   or
+     * +CMGL: <index>, <status>, <length>
+     *
+     * We just read <index>, <stat> and the PDU itself.
+     */
+    r = g_regex_new ("\\+CMGL:\\s*(\\d+)\\s*,\\s*(\\d+)\\s*,(.*)\\r\\n([^\\r\\n]*)(\\r\\n)?",
+                     G_REGEX_RAW | G_REGEX_OPTIMIZE, 0, NULL);
+    g_assert (r != NULL);
+
+    g_regex_match_full (r, str, strlen (str), 0, 0, &match_info, &inner_error);
+    while (!inner_error && g_match_info_matches (match_info)) {
+        MM3gppPduInfo *info;
+
+        info = g_new0 (MM3gppPduInfo, 1);
+        if (mm_get_int_from_match_info (match_info, 1, &info->index) &&
+            mm_get_int_from_match_info (match_info, 2, &info->status) &&
+            (info->pdu = mm_get_string_unquoted_from_match_info (match_info, 4)) != NULL) {
+            /* Append to our list of results and keep on */
+            list = g_list_append (list, info);
+            g_match_info_next (match_info, &inner_error);
+        } else {
+            inner_error = g_error_new (MM_CORE_ERROR,
+                                       MM_CORE_ERROR_FAILED,
+                                       "Error parsing +CMGL response: '%s'",
+                                       str);
+        }
+    }
+
+    g_match_info_free (match_info);
+    g_regex_unref (r);
+
+    if (inner_error) {
+        g_propagate_error (error, inner_error);
+        mm_3gpp_pdu_info_list_free (list);
+        return NULL;
+    }
+
+    return list;
+}
+
+/*************************************************************************/
+
 /* Map two letter facility codes into flag values. There are
  * many more facilities defined (for various flavors of call
  * barring); we only map the ones we care about. */
@@ -2102,8 +2169,8 @@ mm_cdma_normalize_band (const gchar *long_band,
 }
 
 /*************************************************************************/
-
 /* Caller must strip any "+GSN:" or "+CGSN" from @gsn */
+
 gboolean
 mm_parse_gsn (const char *gsn,
               gchar **out_imei,
@@ -2220,4 +2287,3 @@ mm_parse_gsn (const char *gsn,
 
     return success;
 }
-
diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h
index aa646993..eb02337e 100644
--- a/src/mm-modem-helpers.h
+++ b/src/mm-modem-helpers.h
@@ -150,6 +150,17 @@ gint         mm_3gpp_cind_response_get_max       (MM3gppCindResponse *r);
 GByteArray *mm_3gpp_parse_cind_read_response (const gchar *reply,
                                               GError **error);
 
+/* AT+CMGL=4 (list sms parts) response parser */
+typedef struct {
+    gint index;
+    gint status;
+    gchar *pdu;
+} MM3gppPduInfo;
+void   mm_3gpp_pdu_info_list_free      (GList *info_list);
+GList *mm_3gpp_parse_pdu_cmgl_response (const gchar *str,
+                                        GError **error);
+
+
 /* Additional 3GPP-specific helpers */
 
 MMModem3gppFacility mm_3gpp_acronym_to_facility (const gchar *str);
diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
index a6e15db8..6666bb63 100644
--- a/src/tests/test-modem-helpers.c
+++ b/src/tests/test-modem-helpers.c
@@ -22,6 +22,173 @@
 #include "mm-log.h"
 
 /*****************************************************************************/
+/* Test CMGL responses */
+
+static void
+test_cmgl_response (const gchar *str,
+                    const MM3gppPduInfo *expected,
+                    guint n_expected)
+{
+    guint i;
+    GList *list;
+    GError *error = NULL;
+
+    list = mm_3gpp_parse_pdu_cmgl_response (str, &error);
+    g_assert_no_error (error);
+    g_assert (list != NULL);
+    g_assert_cmpuint (g_list_length (list), ==, n_expected);
+
+    for (i = 0; i < n_expected; i++) {
+        GList *l;
+
+        /* Look for the pdu with the expected index */
+        for (l = list; l; l = g_list_next (l)) {
+            MM3gppPduInfo *info = l->data;
+
+            /* Found */
+            if (info->index == expected[i].index) {
+                g_assert_cmpint (info->status, ==, expected[i].status);
+                g_assert_cmpstr (info->pdu, ==, expected[i].pdu);
+                break;
+            }
+        }
+        g_assert (l != NULL);
+    }
+
+    mm_3gpp_pdu_info_list_free (list);
+}
+
+static void
+test_cmgl_response_generic (void *f, gpointer d)
+{
+    const gchar *str =
+        "+CMGL: 0,1,,147\r\n07914306073011F00405812261F700003130916191314095C27"
+        "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+        "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+        "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+        "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07";
+
+    const MM3gppPduInfo expected [] = {
+        {
+            .index = 0,
+            .status = 1,
+            .pdu = "07914306073011F00405812261F700003130916191314095C27"
+            "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+            "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+            "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+            "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"
+        }
+    };
+
+    test_cmgl_response (str, expected, G_N_ELEMENTS (expected));
+}
+
+static void
+test_cmgl_response_generic_multiple (void *f, gpointer d)
+{
+    const gchar *str =
+        "+CMGL: 0,1,,147\r\n07914306073011F00405812261F700003130916191314095C27"
+        "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+        "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+        "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+        "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07\r\n"
+        "+CMGL: 1,1,,147\r\n07914306073011F00405812261F700003130916191314095C27"
+        "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+        "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+        "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+        "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07\r\n"
+        "+CMGL: 2,1,,147\r\n07914306073011F00405812261F700003130916191314095C27"
+        "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+        "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+        "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+        "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07";
+
+    const MM3gppPduInfo expected [] = {
+        {
+            .index = 0,
+            .status = 1,
+            .pdu = "07914306073011F00405812261F700003130916191314095C27"
+            "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+            "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+            "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+            "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"
+        },
+        {
+            .index = 1,
+            .status = 1,
+            .pdu = "07914306073011F00405812261F700003130916191314095C27"
+            "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+            "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+            "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+            "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"
+        },
+        {
+            .index = 2,
+            .status = 1,
+            .pdu = "07914306073011F00405812261F700003130916191314095C27"
+            "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F"
+            "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731"
+            "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B"
+            "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"
+        }
+    };
+
+    test_cmgl_response (str, expected, G_N_ELEMENTS (expected));
+}
+
+static void
+test_cmgl_response_pantech (void *f, gpointer d)
+{
+    const gchar *str =
+        "+CMGL: 17,3,35\r\n079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020";
+
+    const MM3gppPduInfo expected [] = {
+        {
+            .index = 17,
+            .status = 3,
+            .pdu = "079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020"
+        }
+    };
+
+    test_cmgl_response (str, expected, G_N_ELEMENTS (expected));
+}
+
+static void
+test_cmgl_response_pantech_multiple (void *f, gpointer d)
+{
+    const gchar *str =
+        "+CMGL: 17,3,35\r\n079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020\r\n"
+        "+CMGL: 15,3,35\r\n079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020\r\n"
+        "+CMGL: 13,3,35\r\n079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020\r\n"
+        "+CMGL: 11,3,35\r\n079100F40D1101000F001000B917118336058F300";
+
+    const MM3gppPduInfo expected [] = {
+        {
+            .index = 17,
+            .status = 3,
+            .pdu = "079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020"
+        },
+        {
+            .index = 15,
+            .status = 3,
+            .pdu = "079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020"
+        },
+        {
+            .index = 13,
+            .status = 3,
+            .pdu = "079100F40D1101000F001000B917118336058F300001954747A0E4ACF41F27298CDCE83C6EF371B0402814020"
+        },
+        {
+            .index = 11,
+            .status = 3,
+            .pdu = "079100F40D1101000F001000B917118336058F300"
+        }
+    };
+
+    test_cmgl_response (str, expected, G_N_ELEMENTS (expected));
+}
+
+/*****************************************************************************/
 /* Test COPS responses */
 
 static void
@@ -1784,6 +1951,11 @@ int main (int argc, char **argv)
 
     g_test_suite_add (suite, TESTCASE (test_cdma_parse_gsn, NULL));
 
+    g_test_suite_add (suite, TESTCASE (test_cmgl_response_generic, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cmgl_response_generic_multiple, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech_multiple, NULL));
+
     result = g_test_run ();
 
     reg_test_data_free (reg_data);