[PATCH v3] Fixed wrong MEM1 value passed to +CPMS

[Carlo Lobrano]

In functions

- mm_broadband_modem_lock_sms_storages
- modem_messaging_set_default_storage

return with error when using current_sms_mem1_storage as +CPMS value, but
current_sms_mem1_storage value is UNKNOWN.

---

 src/mm-broadband-modem.c       | 79 ++++++++++++++++++++++++++----------------
 src/mm-iface-modem-messaging.c |  2 +-
 2 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/src/mm-broadband-modem.c b/src/mm-broadband-modem.c
index 302fc3d..c15f18d 100644
--- a/src/mm-broadband-modem.c
+++ b/src/mm-broadband-modem.c
@@ -5459,7 +5459,7 @@ mm_broadband_modem_lock_sms_storages (MMBroadbandModem *self,
                                       gpointer user_data)
 {
     LockSmsStoragesContext *ctx;
-    gchar *cmd;
+    gchar *cmd = NULL;
     gchar *mem1_str = NULL;
     gchar *mem2_str = NULL;
 
@@ -5488,45 +5488,55 @@ mm_broadband_modem_lock_sms_storages (MMBroadbandModem *self,
                                              user_data,
                                              mm_broadband_modem_lock_sms_storages);
 
+    /* Some modems may not support empty string parameters, then if mem1 is
+     * UNKNOWN, and current sms mem1 storage has a valid value, we send again
+     * the already locked mem1 value in place of an empty string.
+     * This way we also avoid to confuse the environment of other sync operation
+     * that have potentially locked mem1 previoulsy.
+     */
     if (mem1 != MM_SMS_STORAGE_UNKNOWN) {
         ctx->mem1_locked = TRUE;
         ctx->previous_mem1 = self->priv->current_sms_mem1_storage;
-        self->priv->mem1_storage_locked = TRUE;
+
         self->priv->current_sms_mem1_storage = mem1;
-        mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
+        self->priv->mem1_storage_locked = TRUE;
+    } else if (self->priv->current_sms_mem1_storage != MM_SMS_STORAGE_UNKNOWN) {
+        mm_dbg ("Given sms mem1 storage is unknown. Using current sms mem1 storage value '%s' instead",
+                mm_sms_storage_get_string (self->priv->current_sms_mem1_storage));
+    } else {
+        g_simple_async_report_error_in_idle (
+                G_OBJECT (self),
+                callback,
+                user_data,
+                MM_CORE_ERROR,
+                MM_CORE_ERROR_RETRY,
+                "Unknown SMS mem1 storage is allowed only if the current sms mem1 storage value is not unknown.");
+        lock_sms_storages_context_complete_and_free (ctx);
+        return;
     }
+    mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
 
     if (mem2 != MM_SMS_STORAGE_UNKNOWN) {
         ctx->mem2_locked = TRUE;
         ctx->previous_mem2 = self->priv->current_sms_mem2_storage;
+
         self->priv->mem2_storage_locked = TRUE;
         self->priv->current_sms_mem2_storage = mem2;
-        mem2_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem2_storage), -1);
 
-        if (mem1 == MM_SMS_STORAGE_UNKNOWN) {
-            /* Some modems may not support empty string parameters. Then if mem1 is
-             * UNKNOWN, we send again the already locked mem1 value in place of an
-             * empty string. This way we also avoid to confuse the environment of
-             * other async operation that have potentially locked mem1 previoulsy.
-             * */
-            mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
-        }
+        mem2_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem2_storage), -1);
     }
 
-    /* We don't touch 'mem3' here */
+    g_assert(mem1_str != NULL);
 
+    /* We don't touch 'mem3' here */
     mm_dbg ("Locking SMS storages to: mem1 (%s), mem2 (%s)...",
-            mem1_str ? mem1_str : "none",
+            mem1_str,
             mem2_str ? mem2_str : "none");
 
     if (mem2_str)
-        cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\"",
-                               mem1_str ? mem1_str : "",
-                               mem2_str);
-    else if (mem1_str)
-        cmd = g_strdup_printf ("+CPMS=\"%s\"", mem1_str);
+        cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\"", mem1_str, mem2_str);
     else
-        g_assert_not_reached ();
+        cmd = g_strdup_printf ("+CPMS=\"%s\"", mem1_str);
 
     mm_base_modem_at_command (MM_BASE_MODEM (self),
                               cmd,
@@ -5578,22 +5588,31 @@ modem_messaging_set_default_storage (MMIfaceModemMessaging *_self,
     gchar *mem1_str;
     gchar *mem_str;
 
-    result = g_simple_async_result_new (G_OBJECT (self),
-                                        callback,
-                                        user_data,
-                                        modem_messaging_set_default_storage);
+    /* We provide the current sms storage for mem1 if not UNKNOWN */
+    if (self->priv->current_sms_mem1_storage == MM_SMS_STORAGE_UNKNOWN) {
+        g_simple_async_report_error_in_idle (G_OBJECT (self),
+                                             callback,
+                                             user_data,
+                                             MM_CORE_ERROR,
+                                             MM_CORE_ERROR_INVALID_ARGS,
+                                             "Current SMS MEM1 storage is UNKNOWN (SIM busy?)");
+        return;
+    }
 
     /* Set defaults as current */
     self->priv->current_sms_mem2_storage = storage;
 
-    /* We provide the current sms storage for mem1 if not UNKNOWN */
     mem1_str = g_ascii_strup (mm_sms_storage_get_string (self->priv->current_sms_mem1_storage), -1);
-
     mem_str = g_ascii_strup (mm_sms_storage_get_string (storage), -1);
-    cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"",
-                           mem1_str ? mem1_str : "",
-                           mem_str,
-                           mem_str);
+
+    cmd = g_strdup_printf ("+CPMS=\"%s\",\"%s\",\"%s\"", mem1_str, mem_str, mem_str);
+
+    result = g_simple_async_result_new (G_OBJECT (self),
+                                        callback,
+                                        user_data,
+                                        modem_messaging_set_default_storage);
+
+
     mm_base_modem_at_command (MM_BASE_MODEM (self),
                               cmd,
                               3,
diff --git a/src/mm-iface-modem-messaging.c b/src/mm-iface-modem-messaging.c
index 0cff1f2..01a30f0 100644
--- a/src/mm-iface-modem-messaging.c
+++ b/src/mm-iface-modem-messaging.c
@@ -789,7 +789,7 @@ set_default_storage_ready (MMIfaceModemMessaging *self,
     GError *error = NULL;
 
     if (!MM_IFACE_MODEM_MESSAGING_GET_INTERFACE (self)->set_default_storage_finish (self, res, &error)) {
-        mm_dbg ("Couldn't set default storage: '%s'", error->message);
+        mm_warn ("Could not set default storage: '%s'", error->message);
         g_error_free (error);
     }
 
-- 
2.9.3