Merge request !1341 from 'mbim-multipart-fix'

sms: always use SMS_PART_INVALID_INDEX unstored parts

https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/merge_requests/1341

Closes #979

1 files changed, 83 insertions, 52 deletions

diff --git a/src/mm-sms-list.c b/src/mm-sms-list.c
index f2401dae..30ce1cc4 100644
--- a/src/mm-sms-list.c
+++ b/src/mm-sms-list.c
@@ -24,7 +24,6 @@
 #define _LIBMM_INSIDE_MM
 #include <libmm-glib.h>
 
-#include "mm-iface-modem-messaging.h"
 #include "mm-sms-list.h"
 #include "mm-base-sms.h"
 #include "mm-log-object.h"
@@ -40,10 +39,8 @@ G_DEFINE_TYPE_EXTENDED (MMSmsList, mm_sms_list, G_TYPE_OBJECT, 0,
 enum {
     PROP_0,
     PROP_BIND_TO,
-    PROP_MODEM,
     PROP_LAST
 };
-static GParamSpec *properties[PROP_LAST];
 
 enum {
     SIGNAL_ADDED,
@@ -55,12 +52,12 @@ static guint signals[SIGNAL_LAST];
 struct _MMSmsListPrivate {
     /* The object this SMS list is bound to */
     GObject *bind_to;
-    /* The owner modem */
-    MMBaseModem *modem;
     /* List of sms objects */
     GList *list;
 };
 
+static void _release_sms_internal (MMBaseSms *sms, MMSmsList *self);
+
 /*****************************************************************************/
 
 gboolean
@@ -161,7 +158,7 @@ delete_ready (MMBaseSms *sms,
                             path,
                             (GCompareFunc)cmp_sms_by_path);
     if (l) {
-        g_object_unref (MM_BASE_SMS (l->data));
+        _release_sms_internal (MM_BASE_SMS (l->data), self);
         self->priv->list = g_list_delete_link (self->priv->list, l);
     }
 
@@ -213,14 +210,61 @@ mm_sms_list_delete_sms (MMSmsList *self,
 
 /*****************************************************************************/
 
-void
-mm_sms_list_add_sms (MMSmsList *self,
-                     MMBaseSms *sms)
+static void
+set_local_multipart_reference (MMBaseSms   *sms,
+                               const gchar *number,
+                               MMSmsList   *self)
+{
+    guint8 reference;
+    guint8 first;
+
+    /* Start by looking for a random number */
+    reference = g_random_int_range (1, 255);
+
+    /* Then, look for the given reference in user-created messages */
+    first = reference;
+    do {
+        if (!mm_sms_list_has_local_multipart_reference (self, number, reference)) {
+            mm_base_sms_set_multipart_reference (sms, reference);
+            return;
+        }
+
+        if (reference == 255)
+            reference = 1;
+        else
+            reference++;
+    } while (reference != first);
+}
+
+static void
+_release_sms_internal (MMBaseSms *sms, MMSmsList *self)
+{
+    g_signal_handlers_disconnect_by_func (sms, set_local_multipart_reference, self);
+    g_object_unref (sms);
+}
+
+static void
+_add_sms_internal (MMSmsList *self,
+                   MMBaseSms *sms,
+                   gboolean   received)
 {
     self->priv->list = g_list_prepend (self->priv->list, g_object_ref (sms));
+    g_signal_connect (sms,
+                      MM_BASE_SMS_SET_LOCAL_MULTIPART_REFERENCE,
+                      (GCallback)set_local_multipart_reference,
+                      self);
+
     g_signal_emit (self, signals[SIGNAL_ADDED], 0,
                    mm_base_sms_get_path (sms),
-                   FALSE);
+                   received);
+}
+
+
+void
+mm_sms_list_add_sms (MMSmsList *self,
+                     MMBaseSms *sms)
+{
+    _add_sms_internal (self, sms, FALSE);
 }
 
 /*****************************************************************************/
@@ -259,39 +303,33 @@ cmp_sms_by_part_index_and_storage (MMBaseSms *sms,
 
 static gboolean
 take_singlepart (MMSmsList *self,
+                 MMBaseSms *sms,
                  MMSmsPart *part,
                  MMSmsState state,
                  MMSmsStorage storage,
                  GError **error)
 {
-    MMBaseSms *sms;
-
-    sms = mm_base_sms_singlepart_new (self->priv->modem,
+    if (!mm_base_sms_singlepart_init (sms,
                                       state,
                                       storage,
                                       part,
-                                      error);
-    if (!sms)
+                                      error))
         return FALSE;
 
     mm_obj_dbg (sms, "creating new singlepart SMS object");
-
-    self->priv->list = g_list_prepend (self->priv->list, sms);
-    g_signal_emit (self, signals[SIGNAL_ADDED], 0,
-                   mm_base_sms_get_path (sms),
-                   state == MM_SMS_STATE_RECEIVED);
+    _add_sms_internal (self, sms, state == MM_SMS_STATE_RECEIVED);
     return TRUE;
 }
 
 static gboolean
 take_multipart (MMSmsList *self,
+                MMBaseSms *sms,
                 MMSmsPart *part,
                 MMSmsState state,
                 MMSmsStorage storage,
                 GError **error)
 {
     GList *l;
-    MMBaseSms *sms;
     guint concat_reference;
 
     concat_reference = mm_sms_part_get_concat_reference (part);
@@ -304,25 +342,19 @@ take_multipart (MMSmsList *self,
     }
 
     /* Create new Multipart */
-    sms = mm_base_sms_multipart_new (self->priv->modem,
+    if (!mm_base_sms_multipart_init (sms,
                                      state,
                                      storage,
                                      concat_reference,
                                      mm_sms_part_get_concat_max (part),
                                      part,
-                                     error);
-    if (!sms)
+                                     error))
         return FALSE;
 
     mm_obj_dbg (sms, "creating new multipart SMS object: need to receive %u parts with reference '%u'",
                 mm_sms_part_get_concat_max (part),
                 concat_reference);
-    self->priv->list = g_list_prepend (self->priv->list, sms);
-    g_signal_emit (self, signals[SIGNAL_ADDED], 0,
-                   mm_base_sms_get_path (sms),
-                   (state == MM_SMS_STATE_RECEIVED ||
-                    state == MM_SMS_STATE_RECEIVING));
-
+    _add_sms_internal (self, sms, (state == MM_SMS_STATE_RECEIVED || state == MM_SMS_STATE_RECEIVING));
     return TRUE;
 }
 
@@ -347,6 +379,7 @@ mm_sms_list_has_part (MMSmsList *self,
 
 gboolean
 mm_sms_list_take_part (MMSmsList *self,
+                       MMBaseSms *uninitialized_sms,
                        MMSmsPart *part,
                        MMSmsState state,
                        MMSmsStorage storage,
@@ -379,7 +412,7 @@ mm_sms_list_take_part (MMSmsList *self,
                         mm_sms_part_get_concat_sequence (part),
                         mm_sms_part_get_concat_max (part));
 
-        return take_multipart (self, part, state, storage, error);
+        return take_multipart (self, uninitialized_sms, part, state, storage, error);
     }
 
     /* Otherwise, we build a whole new single-part MMSms just from this part */
@@ -389,7 +422,22 @@ mm_sms_list_take_part (MMSmsList *self,
                     mm_sms_part_get_index (part));
     else
         mm_obj_dbg (self, "SMS part (not stored) is from a singlepart SMS");
-    return take_singlepart (self, part, state, storage, error);
+    return take_singlepart (self, uninitialized_sms, part, state, storage, error);
+}
+
+/*****************************************************************************/
+
+void
+mm_sms_list_set_default_storage (MMSmsList    *self,
+                                 MMSmsStorage  default_storage)
+{
+    GList *l;
+
+    for (l = self->priv->list; l; l = g_list_next (l)) {
+        g_object_set (MM_BASE_SMS (l->data),
+                      MM_BASE_SMS_DEFAULT_STORAGE, default_storage,
+                      NULL);
+    }
 }
 
 /*****************************************************************************/
@@ -403,11 +451,10 @@ log_object_build_id (MMLogObject *_self)
 /*****************************************************************************/
 
 MMSmsList *
-mm_sms_list_new (MMBaseModem *modem, GObject *bind_to)
+mm_sms_list_new (GObject *bind_to)
 {
     /* Create the object */
     return g_object_new  (MM_TYPE_SMS_LIST,
-                          MM_SMS_LIST_MODEM, modem,
                           MM_BIND_TO, bind_to,
                           NULL);
 }
@@ -426,10 +473,6 @@ set_property (GObject *object,
         self->priv->bind_to = g_value_dup_object (value);
         mm_bind_to (MM_BIND (self), NULL, self->priv->bind_to);
         break;
-    case PROP_MODEM:
-        g_clear_object (&self->priv->modem);
-        self->priv->modem = g_value_dup_object (value);
-        break;
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
         break;
@@ -448,9 +491,6 @@ get_property (GObject *object,
     case PROP_BIND_TO:
         g_value_set_object (value, self->priv->bind_to);
         break;
-    case PROP_MODEM:
-        g_value_set_object (value, self->priv->modem);
-        break;
     default:
         G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
         break;
@@ -471,10 +511,9 @@ dispose (GObject *object)
 {
     MMSmsList *self = MM_SMS_LIST (object);
 
-    g_clear_object (&self->priv->modem);
     g_clear_object (&self->priv->bind_to);
-    g_list_free_full (self->priv->list, g_object_unref);
-    self->priv->list = NULL;
+    g_list_foreach (self->priv->list, (GFunc)_release_sms_internal, self);
+    g_clear_pointer (&self->priv->list, (GDestroyNotify)g_list_free);
 
     G_OBJECT_CLASS (mm_sms_list_parent_class)->dispose (object);
 }
@@ -503,14 +542,6 @@ mm_sms_list_class_init (MMSmsListClass *klass)
     object_class->dispose = dispose;
 
     /* Properties */
-    properties[PROP_MODEM] =
-        g_param_spec_object (MM_SMS_LIST_MODEM,
-                             "Modem",
-                             "The Modem which owns this SMS list",
-                             MM_TYPE_BASE_MODEM,
-                             G_PARAM_READWRITE);
-    g_object_class_install_property (object_class, PROP_MODEM, properties[PROP_MODEM]);
-
     g_object_class_override_property (object_class, PROP_BIND_TO, MM_BIND_TO);
 
     /* Signals */