modem-helpers: make cpms test parser return error on failure

4 files changed, 47 insertions, 45 deletions

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 21600d77..d2812051 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -6298,13 +6298,10 @@ cpms_format_check_ready (MMBroadbandModem *self,
     if (!mm_3gpp_parse_cpms_test_response (response,
                                            &result->mem1,
                                            &result->mem2,
-                                           &result->mem3)) {
+                                           &result->mem3,
+                                           &error)) {
         supported_storages_result_free (result);
-        g_task_return_new_error (task,
-                                 MM_CORE_ERROR,
-                                 MM_CORE_ERROR_FAILED,
-                                 "Couldn't parse supported storages reply: '%s'",
-                                 response);
+        g_task_return_error (task, error);
         g_object_unref (task);
         return;
     }
diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index e7ee6e20..e12a26b7 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -2942,17 +2942,18 @@ storage_from_str (const gchar *str)
 }
 
 gboolean
-mm_3gpp_parse_cpms_test_response (const gchar *reply,
-                                  GArray **mem1,
-                                  GArray **mem2,
-                                  GArray **mem3)
+mm_3gpp_parse_cpms_test_response (const gchar  *reply,
+                                  GArray      **mem1,
+                                  GArray      **mem2,
+                                  GArray      **mem3,
+                                  GError      **error)
 {
-    GRegex *r;
-    gchar **split;
     guint i;
-    GArray *tmp1 = NULL;
-    GArray *tmp2 = NULL;
-    GArray *tmp3 = NULL;
+    g_autoptr(GRegex) r = NULL;
+    g_autoptr(GArray) tmp1 = NULL;
+    g_autoptr(GArray) tmp2 = NULL;
+    g_autoptr(GArray) tmp3 = NULL;
+    g_auto(GStrv)     split = NULL;
 
     g_assert (mem1 != NULL);
     g_assert (mem2 != NULL);
@@ -2965,9 +2966,9 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
         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);
+        g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                     "Cannot parse +CPMS test response: invalid number of groups (%u != %u)",
+                     g_strv_length (split), N_EXPECTED_GROUPS);
         return FALSE;
     }
 
@@ -3010,29 +3011,20 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
             g_assert_not_reached ();
     }
 
-    g_strfreev (split);
-    g_regex_unref (r);
-
-    g_warn_if_fail (tmp1 != NULL);
-    g_warn_if_fail (tmp2 != NULL);
-    g_warn_if_fail (tmp3 != NULL);
-
     /* 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;
-        *mem3 = tmp3;
+        *mem1 = g_steal_pointer (&tmp1);
+        *mem2 = g_steal_pointer (&tmp2);
+        *mem3 = g_steal_pointer (&tmp3);
         return TRUE;
     }
 
-    /* Otherwise, cleanup and return FALSE */
-    if (tmp1)
-        g_array_unref (tmp1);
-    if (tmp2)
-        g_array_unref (tmp2);
-    if (tmp3)
-        g_array_unref (tmp3);
+    g_set_error (error, MM_CORE_ERROR, MM_CORE_ERROR_FAILED,
+                 "Cannot parse +CPMS test response: mem1 %s, mem2 %s, mem3 %s",
+                 tmp1 ? "yes" : "no",
+                 tmp2 ? "yes" : "no",
+                 tmp3 ? "yes" : "no");
     return FALSE;
 }
 
diff --git a/src/mm-modem-helpers.h b/src/mm-modem-helpers.h
index 316b26d3..3ad03ef8 100644
--- a/src/mm-modem-helpers.h
+++ b/src/mm-modem-helpers.h
@@ -234,10 +234,11 @@ gboolean mm_3gpp_parse_cmgf_test_response (const gchar *reply,
                                            GError **error);
 
 /* AT+CPMS=? (Preferred SMS storage) response parser */
-gboolean mm_3gpp_parse_cpms_test_response (const gchar *reply,
-                                           GArray **mem1,
-                                           GArray **mem2,
-                                           GArray **mem3);
+gboolean mm_3gpp_parse_cpms_test_response (const gchar  *reply,
+                                           GArray      **mem1,
+                                           GArray      **mem2,
+                                           GArray      **mem3,
+                                           GError      **error);
 
 /* AT+CPMS? (Current SMS storage) response parser */
 gboolean mm_3gpp_parse_cpms_query_response (const gchar *reply,
diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
index a01b0fd9..3a4fdd63 100644
--- a/src/tests/test-modem-helpers.c
+++ b/src/tests/test-modem-helpers.c
@@ -2901,10 +2901,12 @@ test_cpms_response_cinterion (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing Cinterion +CPMS=? response...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     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));
@@ -2929,10 +2931,12 @@ test_cpms_response_huawei_mu609 (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing Huawei MU609 +CPMS=? response...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     g_assert_cmpuint (mem1->len, ==, 1);
     g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
     g_assert_cmpuint (mem2->len, ==, 1);
@@ -2953,10 +2957,12 @@ test_cpms_response_nokia_c6 (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing Nokia C6 response...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     g_assert_cmpuint (mem1->len, ==, 0);
     g_assert_cmpuint (mem2->len, ==, 0);
     g_assert_cmpuint (mem3->len, ==, 0);
@@ -2978,10 +2984,12 @@ test_cpms_response_mixed (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing mixed +CPMS=? response...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     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));
@@ -3003,10 +3011,12 @@ test_cpms_response_mixed_spaces (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing mixed +CPMS=? response with spaces...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     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));
@@ -3032,10 +3042,12 @@ test_cpms_response_empty_fields (void *f, gpointer d)
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
+    GError *error = NULL;
 
     g_debug ("Testing mixed +CPMS=? response...");
 
-    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3));
+    g_assert (mm_3gpp_parse_cpms_test_response (reply, &mem1, &mem2, &mem3, &error));
+    g_assert_no_error (error);
     g_assert_cmpuint (mem1->len, ==, 0);
     g_assert_cmpuint (mem2->len, ==, 0);
     g_assert_cmpuint (mem3->len, ==, 0);