From eaf65ed98dd7e04b985be6b4241e9adab66750aa Mon Sep 17 00:00:00 2001 From: Nick Stevens Date: Thu, 17 Sep 2015 21:31:28 +0000 Subject: [PATCH] core: process SMS +CGMR response with regex Variability in the response style from certain modems causes the parsing of the +CGMR response to fail. For example, the Telit HE910 inserts an empty string ("") in the second field of the response, causing the sscanf implementation to fail. This patch converts the parsing of the CGMR response to a regex that allows for more flexibility and robustness, and adds unit tests around the parsing call. Signed-off-by: Nick Stevens --- src/mm-broadband-modem.c | 17 ++++---- src/mm-modem-helpers.c | 73 +++++++++++++++++++++++++++++++++- src/mm-modem-helpers.h | 5 +++ src/mm-sms-part-3gpp.h | 2 - src/tests/test-modem-helpers.c | 64 +++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 13 deletions(-) diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c index 611a80aa..90b5780d 100644 --- a/src/mm-broadband-modem.c +++ b/src/mm-broadband-modem.c @@ -5614,8 +5614,7 @@ sms_part_ready (MMBroadbandModem *self, SmsPartContext *ctx) { MMSmsPart *part; - gint rv, status, tpdu_len; - gchar pdu[MM_SMS_PART_3GPP_MAX_PDU_LEN + 1]; + MM3gppPduInfo *info; const gchar *response; GError *error = NULL; @@ -5633,19 +5632,16 @@ sms_part_ready (MMBroadbandModem *self, return; } - rv = sscanf (response, "+CMGR: %d,,%d %" G_STRINGIFY (MM_SMS_PART_3GPP_MAX_PDU_LEN) "s", - &status, &tpdu_len, pdu); - if (rv != 3) { - error = g_error_new (MM_CORE_ERROR, - MM_CORE_ERROR_FAILED, - "Failed to parse CMGR response (parsed %d items)", rv); - mm_warn ("Couldn't retrieve SMS part: '%s'", error->message); + info = mm_3gpp_parse_cmgr_read_response (response, ctx->idx, &error); + if (!info) { + mm_warn ("Couldn't parse SMS part: '%s'", + error->message); g_simple_async_result_take_error (ctx->result, error); sms_part_context_complete_and_free (ctx); return; } - part = mm_sms_part_3gpp_new_from_pdu (ctx->idx, pdu, &error); + part = mm_sms_part_3gpp_new_from_pdu (info->index, info->pdu, &error); if (part) { mm_dbg ("Correctly parsed PDU (%d)", ctx->idx); mm_iface_modem_messaging_take_part (MM_IFACE_MODEM_MESSAGING (self), @@ -5659,6 +5655,7 @@ sms_part_ready (MMBroadbandModem *self, } /* All done */ + mm_3gpp_pdu_info_free (info); g_simple_async_result_set_op_res_gboolean (ctx->result, TRUE); sms_part_context_complete_and_free (ctx); } diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c index fb0b7fa1..5c9c269c 100644 --- a/src/mm-modem-helpers.c +++ b/src/mm-modem-helpers.c @@ -1201,6 +1201,77 @@ mm_3gpp_parse_cmgf_test_response (const gchar *reply, /*************************************************************************/ +MM3gppPduInfo * +mm_3gpp_parse_cmgr_read_response (const gchar *reply, + guint index, + GError **error) +{ + GRegex *r; + GMatchInfo *match_info; + gint count; + gint status; + gchar *pdu; + MM3gppPduInfo *info = NULL; + + /* +CMGR: ,,(whitespace) */ + /* The and fields are matched, but not currently used */ + r = g_regex_new ("\\+CMGR:\\s*(\\d+)\\s*,([^,]*),\\s*(\\d+)\\s*([^\\r\\n]*)", 0, 0, NULL); + g_assert (r); + + if (!g_regex_match_full (r, reply, strlen (reply), 0, 0, &match_info, NULL)) { + g_set_error (error, + MM_CORE_ERROR, + MM_CORE_ERROR_FAILED, + "Failed to parse CMGR read result: response didn't match '%s'", + reply); + goto done; + } + + /* g_match_info_get_match_count includes match #0 */ + if ((count = g_match_info_get_match_count (match_info)) != 5) { + g_set_error (error, + MM_CORE_ERROR, + MM_CORE_ERROR_FAILED, + "Failed to match CMGR fields (matched %d) '%s'", + count, + reply); + goto done; + } + + if (!mm_get_int_from_match_info (match_info, 1, &status)) { + g_set_error (error, + MM_CORE_ERROR, + MM_CORE_ERROR_FAILED, + "Failed to extract CMGR status field '%s'", + reply); + goto done; + } + + + pdu = mm_get_string_unquoted_from_match_info (match_info, 4); + if (!pdu) { + g_set_error (error, + MM_CORE_ERROR, + MM_CORE_ERROR_FAILED, + "Failed to extract CMGR pdu field '%s'", + reply); + goto done; + } + + info = g_new0 (MM3gppPduInfo, 1); + info->index = index; + info->status = status; + info->pdu = pdu; + +done: + g_match_info_free (match_info); + g_regex_unref (r); + + return info; +} + +/*************************************************************************/ + static MMSmsStorage storage_from_str (const gchar *str) { @@ -1698,7 +1769,7 @@ done: /*************************************************************************/ -static void +void mm_3gpp_pdu_info_free (MM3gppPduInfo *info) { g_free (info->pdu); diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h index 9fabd6aa..e92b6c0a 100644 --- a/src/mm-modem-helpers.h +++ b/src/mm-modem-helpers.h @@ -191,10 +191,15 @@ typedef struct { gint status; gchar *pdu; } MM3gppPduInfo; +void mm_3gpp_pdu_info_free (MM3gppPduInfo *info); void mm_3gpp_pdu_info_list_free (GList *info_list); GList *mm_3gpp_parse_pdu_cmgl_response (const gchar *str, GError **error); +/* AT+CMGR (Read message) response parser */ +MM3gppPduInfo *mm_3gpp_parse_cmgr_read_response (const gchar *reply, + guint index, + GError **error); /* Additional 3GPP-specific helpers */ diff --git a/src/mm-sms-part-3gpp.h b/src/mm-sms-part-3gpp.h index 82709a2f..a21d7347 100644 --- a/src/mm-sms-part-3gpp.h +++ b/src/mm-sms-part-3gpp.h @@ -22,8 +22,6 @@ #include "mm-sms-part.h" -#define MM_SMS_PART_3GPP_MAX_PDU_LEN 344 - MMSmsPart *mm_sms_part_3gpp_new_from_pdu (guint index, const gchar *hexpdu, GError **error); diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c index 9fd25016..dfced5ea 100644 --- a/src/tests/test-modem-helpers.c +++ b/src/tests/test-modem-helpers.c @@ -195,6 +195,67 @@ test_cmgl_response_pantech_multiple (void *f, gpointer d) test_cmgl_response (str, expected, G_N_ELEMENTS (expected)); } +/*****************************************************************************/ +/* Test CMGR responses */ + +static void +test_cmgr_response (const gchar *str, + const MM3gppPduInfo *expected) +{ + MM3gppPduInfo *info; + GError *error = NULL; + + info = mm_3gpp_parse_cmgr_read_response (str, 0, &error); + g_assert_no_error (error); + g_assert (info != NULL); + + /* Ignore index, it is not included in CMGR response */ + g_assert_cmpint (info->status, ==, expected->status); + g_assert_cmpstr (info->pdu, ==, expected->pdu); +} + +static void +test_cmgr_response_generic (void *f, gpointer d) +{ + const gchar *str = + "+CMGR: 1,,147 07914306073011F00405812261F700003130916191314095C27" + "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F" + "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731" + "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B" + "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07"; + + const MM3gppPduInfo expected = { + .index = 0, + .status = 1, + .pdu = "07914306073011F00405812261F700003130916191314095C27" + "4D96D2FBBD3E437280CB2BEC961F3DB5D76818EF2F0381D9E83E06F39A8CC2E9FD372F" + "77BEE0249CBE37A594E0E83E2F532085E2F93CB73D0B93CA7A7DFEEB01C447F93DF731" + "0BD3E07CDCB727B7A9C7ECF41E432C8FC96B7C32079189E26874179D0F8DD7E93C3A0B" + "21B246AA641D637396C7EBBCB22D0FD7E77B5D376B3AB3C07" + }; + + test_cmgr_response (str, &expected); +} + +/* Telit HE910 places empty quotation marks in the field and a CR+LF + * before the PDU */ +static void +test_cmgr_response_telit (void *f, gpointer d) +{ + const gchar *str = + "+CMGR: 0,\"\",50\r\n07916163838428F9040B916121021021F7000051905141642" + "20A23C4B0BCFD5E8740C4B0BCFD5E83C26E3248196687C9A0301D440DBBC3677918"; + + const MM3gppPduInfo expected = { + .index = 0, + .status = 0, + .pdu = "07916163838428F9040B916121021021F7000051905141642" + "20A23C4B0BCFD5E8740C4B0BCFD5E83C26E3248196687C9A0301D440DBBC3677918" + }; + + test_cmgr_response (str, &expected); +} + /*****************************************************************************/ /* Test COPS responses */ @@ -2568,6 +2629,9 @@ int main (int argc, char **argv) g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech, NULL)); g_test_suite_add (suite, TESTCASE (test_cmgl_response_pantech_multiple, NULL)); + g_test_suite_add (suite, TESTCASE (test_cmgr_response_generic, NULL)); + g_test_suite_add (suite, TESTCASE (test_cmgr_response_telit, NULL)); + g_test_suite_add (suite, TESTCASE (test_supported_mode_filter, NULL)); g_test_suite_add (suite, TESTCASE (test_supported_capability_filter, NULL));