From 508a37a8916a70831cc74cba1d8b977cf95e4a27 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Wed, 2 Dec 2015 21:38:06 +0100 Subject: =?UTF-8?q?modem-helpers:=20fix=20parsing=20CPMS=3D=3F=20responses?= =?UTF-8?q?=20without=20groups?= 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 --- src/mm-modem-helpers.c | 108 +++++++++++++++++++++++++------- src/tests/test-modem-helpers.c | 137 +++++++++++++++++++++++++++++++++++++++-- 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)); -- cgit v1.2.1