modem-helpers: fix parsing CPMS=? responses without groups

The CPMS test parser was expecting 3 groups (parenthesis enclosed lists) of
memory IDs, e.g.:

   +CPMS: ("SM","ME"),("SM","ME"),("SM","ME")

But some modems like the Huawei MU609 may just report single elements, not
groups, e.g.:

   +CPMS: "SM","SM","SM"

This patch avoids using g_strsplit() to split the groups, as that is unaware
of the possible replies without groups. Instead, a new helper method is
implemented which does the group/item split itself, considering also the
possibility of a reply with mixed groups and non-groups, like e.g.:

   +CPMS: ("SM","ME"),"SM","SM"

Additionally, we also now support the case where the groups are empty, e.g.:

   +CPMS: (),(),()

https://bugs.freedesktop.org/show_bug.cgi?id=92243

2 files changed, 220 insertions, 25 deletions

diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index 5c9c269cb..19fe81ef4 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -1290,6 +1290,67 @@ storage_from_str (const gchar *str)
     return MM_SMS_STORAGE_UNKNOWN;
 }
 
+static gchar **
+helper_split_groups (const gchar *str)
+{
+    GPtrArray *array;
+    const gchar *start;
+    const gchar *next;
+
+    array = g_ptr_array_new ();
+
+    /*
+     * Manually parse splitting groups. Groups may be single elements, or otherwise
+     * lists given between parenthesis, e.g.:
+     *
+     *    ("SM","ME"),("SM","ME"),("SM","ME")
+     *    "SM","SM","SM"
+     *    "SM",("SM","ME"),("SM","ME")
+     */
+
+    /* Iterate string splitting groups */
+    for (start = str; start; start = next) {
+        gchar *item;
+        gssize len = -1;
+
+        /* skip leading whitespaces */
+        while (*start == ' ')
+            start++;
+
+        if (*start == '(') {
+            start++;
+            next = strchr (start, ')');
+            if (next) {
+                len = next - start;
+                next = strchr (next, ',');
+                if (next)
+                    next++;
+            }
+        } else {
+            next = strchr (start, ',');
+            if (next) {
+                len = next - start;
+                next++;
+            }
+        }
+
+        if (len < 0)
+            item = g_strdup (start);
+        else
+            item = g_strndup (start, len);
+
+        g_ptr_array_add (array, item);
+    }
+
+    if (array->len > 0) {
+        g_ptr_array_add (array, NULL);
+        return (gchar **) g_ptr_array_free (array, FALSE);
+    }
+
+    g_ptr_array_unref (array);
+    return NULL;
+}
+
 gboolean
 mm_3gpp_parse_cpms_test_response (const gchar *reply,
                                   GArray **mem1,
@@ -1307,23 +1368,31 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
     g_assert (mem2 != NULL);
     g_assert (mem3 != NULL);
 
-    /*
-     * +CPMS: ("SM","ME"),("SM","ME"),("SM","ME")
-     */
-    split = g_strsplit_set (mm_strip_tag (reply, "+CPMS:"), "()", -1);
+#define N_EXPECTED_GROUPS 3
+
+    split = helper_split_groups (mm_strip_tag (reply, "+CPMS:"));
     if (!split)
         return FALSE;
 
+    if (g_strv_length (split) != N_EXPECTED_GROUPS) {
+        mm_warn ("Cannot parse +CPMS test response: invalid number of groups (%u != %u)",
+                 g_strv_length (split), N_EXPECTED_GROUPS);
+        g_strfreev (split);
+        return FALSE;
+    }
+
     r = g_regex_new ("\\s*\"([^,\\)]+)\"\\s*", 0, 0, NULL);
     g_assert (r);
 
-    for (i = 0; split[i]; i++) {
-        GMatchInfo *match_info;
+    for (i = 0; i < N_EXPECTED_GROUPS; i++) {
+        GMatchInfo *match_info = NULL;
+        GArray *array;
+
+        /* We always return a valid array, even if it may be empty */
+        array = g_array_new (FALSE, FALSE, sizeof (MMSmsStorage));
 
         /* Got a range group to match */
         if (g_regex_match_full (r, split[i], strlen (split[i]), 0, 0, &match_info, NULL)) {
-            GArray *array = NULL;
-
             while (g_match_info_matches (match_info)) {
                 gchar *str;
 
@@ -1331,9 +1400,6 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
                 if (str) {
                     MMSmsStorage storage;
 
-                    if (!array)
-                        array = g_array_new (FALSE, FALSE, sizeof (MMSmsStorage));
-
                     storage = storage_from_str (str);
                     g_array_append_val (array, storage);
                     g_free (str);
@@ -1341,18 +1407,17 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
 
                 g_match_info_next (match_info, NULL);
             }
-
-            if (!tmp1)
-                tmp1 = array;
-            else if (!tmp2)
-                tmp2 = array;
-            else if (!tmp3)
-                tmp3 = array;
         }
         g_match_info_free (match_info);
 
-        if (tmp3 != NULL)
-            break; /* once we got the last group, exit... */
+        if (!tmp1)
+            tmp1 = array;
+        else if (!tmp2)
+            tmp2 = array;
+        else if (!tmp3)
+            tmp3 = array;
+        else
+            g_assert_not_reached ();
     }
 
     g_strfreev (split);
@@ -1362,7 +1427,8 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
     g_warn_if_fail (tmp2 != NULL);
     g_warn_if_fail (tmp3 != NULL);
 
-    /* Only return TRUE if all sets have been parsed correctly */
+    /* Only return TRUE if all sets have been parsed correctly
+     * (even if the arrays may be empty) */
     if (tmp1 && tmp2 && tmp3) {
         *mem1 = tmp1;
         *mem2 = tmp2;
diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
index f0706b63a..bd7f68891 100644
--- a/src/tests/test-modem-helpers.c
+++ b/src/tests/test-modem-helpers.c
@@ -1897,14 +1897,14 @@ test_cpms_response_cinterion (void *f, gpointer d)
     trace ("\nTesting Cinterion +CPMS=? response...\n");
 
     g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
-    g_assert (mem1->len == 2);
+    g_assert_cmpuint (mem1->len, ==, 2);
     g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
     g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_MT));
-    g_assert (mem2->len == 3);
+    g_assert_cmpuint (mem2->len, ==, 3);
     g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_ME));
     g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_SM));
     g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_MT));
-    g_assert (mem3->len == 2);
+    g_assert_cmpuint (mem3->len, ==, 2);
     g_assert (is_storage_supported (mem3, MM_SMS_STORAGE_SM));
     g_assert (is_storage_supported (mem3, MM_SMS_STORAGE_MT));
 
@@ -1913,6 +1913,130 @@ test_cpms_response_cinterion (void *f, gpointer d)
     g_array_unref (mem3);
 }
 
+static void
+test_cpms_response_huawei_mu609 (void *f, gpointer d)
+{
+    /* Use different sets for each on purpose, even if weird */
+    const gchar *reply = "+CPMS: \"ME\",\"MT\",\"SM\"";
+    GArray *mem1 = NULL;
+    GArray *mem2 = NULL;
+    GArray *mem3 = NULL;
+
+    trace ("\nTesting Huawei MU609 +CPMS=? response...\n");
+
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert_cmpuint (mem1->len, ==, 1);
+    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
+    g_assert_cmpuint (mem2->len, ==, 1);
+    g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_MT));
+    g_assert_cmpuint (mem3->len, ==, 1);
+    g_assert (is_storage_supported (mem3, MM_SMS_STORAGE_SM));
+
+    g_array_unref (mem1);
+    g_array_unref (mem2);
+    g_array_unref (mem3);
+}
+
+static void
+test_cpms_response_nokia_c6 (void *f, gpointer d)
+{
+    /* Use different sets for each on purpose, even if weird */
+    const gchar *reply = "+CPMS: (),(),()";
+    GArray *mem1 = NULL;
+    GArray *mem2 = NULL;
+    GArray *mem3 = NULL;
+
+    trace ("\nTesting Nokia C6 response...\n");
+
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert_cmpuint (mem1->len, ==, 0);
+    g_assert_cmpuint (mem2->len, ==, 0);
+    g_assert_cmpuint (mem3->len, ==, 0);
+
+    g_array_unref (mem1);
+    g_array_unref (mem2);
+    g_array_unref (mem3);
+}
+
+static void
+test_cpms_response_mixed (void *f, gpointer d)
+{
+    /*
+     * First:    ("ME","MT")  2-item group
+     * Second:   "ME"         1 item
+     * Third:    ("SM")       1-item group
+     */
+    const gchar *reply = "+CPMS: (\"ME\",\"MT\"),\"ME\",(\"SM\")";
+    GArray *mem1 = NULL;
+    GArray *mem2 = NULL;
+    GArray *mem3 = NULL;
+
+    trace ("\nTesting mixed +CPMS=? response...\n");
+
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert_cmpuint (mem1->len, ==, 2);
+    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
+    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_MT));
+    g_assert_cmpuint (mem2->len, ==, 1);
+    g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_ME));
+    g_assert_cmpuint (mem3->len, ==, 1);
+    g_assert (is_storage_supported (mem3, MM_SMS_STORAGE_SM));
+
+    g_array_unref (mem1);
+    g_array_unref (mem2);
+    g_array_unref (mem3);
+}
+
+static void
+test_cpms_response_mixed_spaces (void *f, gpointer d)
+{
+    /* Test with whitespaces here and there */
+    const gchar *reply = "+CPMS:     (  \"ME\"  ,  \"MT\"  )   ,  \"ME\" ,   (  \"SM\"  )";
+    GArray *mem1 = NULL;
+    GArray *mem2 = NULL;
+    GArray *mem3 = NULL;
+
+    trace ("\nTesting mixed +CPMS=? response with spaces...\n");
+
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert_cmpuint (mem1->len, ==, 2);
+    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
+    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_MT));
+    g_assert_cmpuint (mem2->len, ==, 1);
+    g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_ME));
+    g_assert_cmpuint (mem3->len, ==, 1);
+    g_assert (is_storage_supported (mem3, MM_SMS_STORAGE_SM));
+
+    g_array_unref (mem1);
+    g_array_unref (mem2);
+    g_array_unref (mem3);
+}
+
+static void
+test_cpms_response_empty_fields (void *f, gpointer d)
+{
+    /*
+     * First:    ()    Empty group
+     * Second:         Empty item
+     * Third:    (  )  Empty group with spaces
+     */
+    const gchar *reply = "+CPMS: (),,(  )";
+    GArray *mem1 = NULL;
+    GArray *mem2 = NULL;
+    GArray *mem3 = NULL;
+
+    trace ("\nTesting mixed +CPMS=? response...\n");
+
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert_cmpuint (mem1->len, ==, 0);
+    g_assert_cmpuint (mem2->len, ==, 0);
+    g_assert_cmpuint (mem3->len, ==, 0);
+
+    g_array_unref (mem1);
+    g_array_unref (mem2);
+    g_array_unref (mem3);
+}
+
 /*****************************************************************************/
 /* Test CNUM responses */
 
@@ -2604,7 +2728,12 @@ int main (int argc, char **argv)
         item++;
     }
 
-    g_test_suite_add (suite, TESTCASE (test_cpms_response_cinterion, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_cinterion,    NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_huawei_mu609, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_nokia_c6,     NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_mixed,        NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_mixed_spaces, NULL));
+    g_test_suite_add (suite, TESTCASE (test_cpms_response_empty_fields, NULL));
 
     g_test_suite_add (suite, TESTCASE (test_cgdcont_test_response_single, NULL));
     g_test_suite_add (suite, TESTCASE (test_cgdcont_test_response_multiple, NULL));