modem-helpers: don't return any result if an error happens when parsing CPMS=?

We shouldn't rely on the caller to cleanup intermediate results when returning
an error.

2 files changed, 33 insertions, 15 deletions

diff --git a/src/mm-modem-helpers.c b/src/mm-modem-helpers.c
index 9f7dd4bc..e4967d70 100644
--- a/src/mm-modem-helpers.c
+++ b/src/mm-modem-helpers.c
@@ -1188,6 +1188,9 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
     GRegex *r;
     gchar **split;
     guint i;
+    GArray *tmp1 = NULL;
+    GArray *tmp2 = NULL;
+    GArray *tmp3 = NULL;
 
     g_assert (mem1 != NULL);
     g_assert (mem2 != NULL);
@@ -1228,27 +1231,42 @@ mm_3gpp_parse_cpms_test_response (const gchar *reply,
                 g_match_info_next (match_info, NULL);
             }
 
-            if (!*mem1)
-                *mem1 = array;
-            else if (!*mem2)
-                *mem2 = array;
-            else if (!*mem3)
-                *mem3 = array;
+            if (!tmp1)
+                tmp1 = array;
+            else if (!tmp2)
+                tmp2 = array;
+            else if (!tmp3)
+                tmp3 = array;
         }
         g_match_info_free (match_info);
 
-        if (*mem3 != NULL)
+        if (tmp3 != NULL)
             break; /* once we got the last group, exit... */
     }
 
     g_strfreev (split);
     g_regex_unref (r);
 
-    g_warn_if_fail (*mem1 != NULL);
-    g_warn_if_fail (*mem2 != NULL);
-    g_warn_if_fail (*mem3 != NULL);
-
-    return (*mem1 && *mem2 && *mem3);
+    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 */
+    if (tmp1 && tmp2 && tmp3) {
+        *mem1 = tmp1;
+        *mem2 = tmp2;
+        *mem3 = 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);
+    return FALSE;
 }
 
 /*************************************************************************/
diff --git a/src/tests/test-modem-helpers.c b/src/tests/test-modem-helpers.c
index 6175a052..b493e6bc 100644
--- a/src/tests/test-modem-helpers.c
+++ b/src/tests/test-modem-helpers.c
@@ -1825,7 +1825,8 @@ is_storage_supported (GArray *supported,
 static void
 test_cpms_response_cinterion (void *f, gpointer d)
 {
-    const gchar *reply = "+CPMS: (\"ME\",\"SM\",\"MT\"),(\"ME\",\"SM\",\"MT\"),(\"SM\",\"MT\")";
+    /* Use different sets for each on purpose, even if weird */
+    const gchar *reply = "+CPMS: (\"ME\",\"MT\"),(\"ME\",\"SM\",\"MT\"),(\"SM\",\"MT\")";
     GArray *mem1 = NULL;
     GArray *mem2 = NULL;
     GArray *mem3 = NULL;
@@ -1833,9 +1834,8 @@ 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 == 3);
+    g_assert (mem1->len == 2);
     g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_ME));
-    g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_SM));
     g_assert (is_storage_supported (mem1, MM_SMS_STORAGE_MT));
     g_assert (mem2->len == 3);
     g_assert (is_storage_supported (mem2, MM_SMS_STORAGE_ME));