Skip to contentSkip to conversation

Persist TX SMS messages

PATCH Superseded Kristen Carlson Accardi 3 participants 13 messages Nov 24, 2010 – Dec 11, 2010

lore
Share & tools
lore

Kristen Carlson Accardi

author
#0 lore raw

cover Persist TX SMS messages

This patch series implements a method of persisting pending outgoing sms messages on disk. Once an sms message has been submitted to the transmit queue, each pdu in the message is stored on disk until the driver indicates that it has been sent.

Each tx_queue_entry is given an id number that represents it's order in the queue. A subdirectory per entry is created as id-uuid, in order to preserve order when the queue is loaded in the event of a crash. Within each subdirectory, each pdu is saved in order by sequence number.

When an sms driver is ready for operation, the disk will be scanned for any saved pending tx sms messages. If there are any found, they will be loaded from disk and placed into the queue in order.

Kristen Carlson Accardi (3):
sms: store pending tx pdus on disk
sms: delete sent sms messages from backup
sms: restore pending tx messages from backup

 src/ofono.h   |    1 +
 src/sms.c     |   64 ++++++++++++++++++++
 src/smsutil.c |  180 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |   13 ++++
 4 files changed, 258 insertions(+), 0 deletions(-)
Signature · 1 line
1.7.2.3
author
#1 lore raw

PATCH 1/3 sms: store pending tx pdus on disk

---
 src/sms.c     |   17 +++++++++++++++++
 src/smsutil.c |   26 ++++++++++++++++++++++++++
 src/smsutil.h |    3 +++
 3 files changed, 46 insertions(+), 0 deletions(-)
src/sms.c +17 -0
index 12988c8..6eab0fa 100644
@@ -52,6 +52,8 @@ static gboolean tx_next(gpointer user_data);
 
 static GSList *g_drivers = NULL;
 
+static unsigned long tx_counter = 0;
+
 enum message_state {
 	MESSAGE_STATE_PENDING =		0,
 	MESSAGE_STATE_SENT,

@@ -111,6 +113,7 @@ struct tx_queue_entry {
 	ofono_sms_txq_submit_cb_t cb;
 	void *data;
 	ofono_destroy_func destroy;
+	int id;
 };
 
 static gboolean uuid_equal(gconstpointer v1, gconstpointer v2)

@@ -1899,6 +1902,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 {
 	struct message *m = NULL;
 	struct tx_queue_entry *entry;
+	int i;
+	GSList *l;
 
 	entry = tx_queue_entry_new(list, flags);
 	if (entry == NULL)

@@ -1923,6 +1928,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 			sms->ref = sms->ref + 1;
 	}
 
+	entry->id = tx_counter++;
+
 	g_queue_push_tail(sms->txq, entry);
 
 	if (g_queue_get_length(sms->txq) == 1)

@@ -1931,6 +1938,16 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 	if (uuid)
 		memcpy(uuid, &entry->uuid, sizeof(*uuid));
 
+	i = 0;
+
+	for (l = list; l; l = l->next) {
+		struct sms *s = l->data;
+		sms_tx_store(sms->imsi, entry->id,
+				ofono_uuid_to_str(&entry->uuid),
+				s, i);
+		i++;
+	}
+
 	if (cb)
 		cb(sms, &entry->uuid, data);
src/smsutil.c +26 -0
index e6dbf5f..4904419 100644
@@ -48,6 +48,10 @@
 #define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
 #define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%s"
 
+#define SMS_TX_BACKUP_PATH STORAGEDIR "/%s/tx_queue"
+#define SMS_TX_BACKUP_PATH_DIR SMS_TX_BACKUP_PATH "/%lu-%s"
+#define SMS_TX_BACKUP_PATH_FILE SMS_TX_BACKUP_PATH_DIR "/%03i"
+
 #define SMS_ADDR_FMT "%24[0-9A-F]"
 #define SMS_MSGID_FMT "%40[0-9A-F]"
 

@@ -3125,6 +3129,28 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
+			struct sms *s, guint8 seq)
+{
+	int len;
+	unsigned char buf[177];
+
+	if (!imsi)
+		return FALSE;
+
+	len = sms_serialize(buf, s);
+
+	/*
+	 * file name is: imsi/order in tx_queue-uuid/order of pdus
+	 */
+	if (write_file(buf, len, SMS_BACKUP_MODE,
+				SMS_TX_BACKUP_PATH_FILE, imsi, id, uuid,
+				seq) != len)
+		return FALSE;
+
+	return TRUE;
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
src/smsutil.h +3 -0
index 4b05313..5279489 100644
@@ -516,6 +516,9 @@ void status_report_assembly_add_fragment(struct status_report_assembly
 void status_report_assembly_expire(struct status_report_assembly *assembly,
 					time_t before);
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
+				struct sms *s, guint8 seq);
+
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
 				gboolean use_delivery_reports);
Signature · 1 line
1.7.2.3
#2 lore raw

Re: [PATCH 1/3] sms: store pending tx pdus on disk

Hi Kristen,

Quoted 11 lines · Kristen Carlson Accardi
---
 src/sms.c     |   17 +++++++++++++++++
 src/smsutil.c |   26 ++++++++++++++++++++++++++
 src/smsutil.h |    3 +++
 3 files changed, 46 insertions(+), 0 deletions(-)
src/sms.c +0 -0
index 12988c8..6eab0fa 100644

src/sms.c static gboolean tx_next(gpointer user_data); · line 56

Quoted 5 lines
src/sms.c +2 -0
@@ -52,6 +52,8 @@ static gboolean tx_next(gpointer user_data);
 
 static GSList *g_drivers = NULL;
 
+static unsigned long tx_counter = 0;
+

I think that using a static variable for this is a bad idea. You probably want to add this to the sim atom itself. More on this a bit later.

src/sms.c struct tx_queue_entry { · line 116

Quoted 8 lines
src/sms.c +1 -0
 enum message_state {
 	MESSAGE_STATE_PENDING =		0,
 	MESSAGE_STATE_SENT,

@@ -111,6 +113,7 @@ struct tx_queue_entry {
 	ofono_sms_txq_submit_cb_t cb;
 	void *data;
 	ofono_destroy_func destroy;
+	int id;

Is there a reason for the type of id and the type of tx_counter to be different?

Quoted 22 lines
src/sms.c +4 -0
 };
 
 static gboolean uuid_equal(gconstpointer v1, gconstpointer v2)

@@ -1899,6 +1902,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 {
 	struct message *m = NULL;
 	struct tx_queue_entry *entry;
+	int i;
+	GSList *l;
 
 	entry = tx_queue_entry_new(list, flags);
 	if (entry == NULL)

@@ -1923,6 +1928,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 			sms->ref = sms->ref + 1;
 	}
 
+	entry->id = tx_counter++;
+
 	g_queue_push_tail(sms->txq, entry);
 
 	if (g_queue_get_length(sms->txq) == 1)

src/sms.c int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, · line 1943

Quoted 6 lines
src/sms.c +3 -0
@@ -1931,6 +1938,16 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 	if (uuid)
 		memcpy(uuid, &entry->uuid, sizeof(*uuid));
 
+	i = 0;
+
+	for (l = list; l; l = l->next) {

I'd really prefer i = 0, l = list here

Quoted 38 lines
src/sms.c +7 -0
+		struct sms *s = l->data;
+		sms_tx_store(sms->imsi, entry->id,
+				ofono_uuid_to_str(&entry->uuid),
+				s, i);
+		i++;
+	}
+
 	if (cb)
 		cb(sms, &entry->uuid, data);
src/smsutil.c +13 -0
index e6dbf5f..4904419 100644
@@ -48,6 +48,10 @@
 #define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
 #define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%s"
 
+#define SMS_TX_BACKUP_PATH STORAGEDIR "/%s/tx_queue"
+#define SMS_TX_BACKUP_PATH_DIR SMS_TX_BACKUP_PATH "/%lu-%s"
+#define SMS_TX_BACKUP_PATH_FILE SMS_TX_BACKUP_PATH_DIR "/%03i"
+
 #define SMS_ADDR_FMT "%24[0-9A-F]"
 #define SMS_MSGID_FMT "%40[0-9A-F]"
 

@@ -3125,6 +3129,28 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
+			struct sms *s, guint8 seq)
+{
+	int len;
+	unsigned char buf[177];
+
+	if (!imsi)
+		return FALSE;
+

src/smsutil.c void status_report_assembly_expire(struct status_report_assembly *assembly, · line 3150

Quoted 10 lines
src/smsutil.c +10 -0
+	len = sms_serialize(buf, s);
+
+	/*
+	 * file name is: imsi/order in tx_queue-uuid/order of pdus
+	 */
+	if (write_file(buf, len, SMS_BACKUP_MODE,
+				SMS_TX_BACKUP_PATH_FILE, imsi, id, uuid,
+				seq) != len)
+		return FALSE;
+

And where are you taking care of storing the other needed information? e.g. the tx_queue entry flags, retry count, etc?

Quoted 11 lines
src/smsutil.c +3 -0
+	return TRUE;
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
src/smsutil.h +0 -0
index 4b05313..5279489 100644

src/smsutil.h void status_report_assembly_add_fragment(struct status_report_assembly · line 524

Quoted 9 lines
src/smsutil.h +3 -0
@@ -516,6 +516,9 @@ void status_report_assembly_add_fragment(struct status_report_assembly
 void status_report_assembly_expire(struct status_report_assembly *assembly,
 					time_t before);
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
+				struct sms *s, guint8 seq);
+
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
 				gboolean use_delivery_reports);

Otherwise I'm happy with this one.

Regards,
-Denis

author
#3 lore raw

Re: [PATCH 1/3] sms: store pending tx pdus on disk

On Mon, 06 Dec 2010 20:38:58 -0600
Denis Kenzior <[email protected]> wrote:

<snip>

Quoted 11 lines

src/smsutil.c void status_report_assembly_expire(struct status_report_assembly *assembly, · line 3150

Quoted 8 lines
src/smsutil.c +8 -0
+	/*
+	 * file name is: imsi/order in tx_queue-uuid/order of pdus
+	 */
+	if (write_file(buf, len, SMS_BACKUP_MODE,
+				SMS_TX_BACKUP_PATH_FILE, imsi, id, uuid,
+				seq) != len)
+		return FALSE;
+

And where are you taking care of storing the other needed information? e.g. the tx_queue entry flags, retry count, etc?

I can easily add the flags and retry count. As far as the "etc" that you mention, the only other information I see would be entry->cb and entry->data, which I was thinking might have to get lost. I am not sure how to persist that information on disk - do you have any suggestions?

#4 lore raw

Re: [PATCH 1/3] sms: store pending tx pdus on disk

Quoted 17 lines · Kristen Carlson Accardi

On Mon, 06 Dec 2010 20:38:58 -0600
Denis Kenzior <[email protected]> wrote:

<snip>

Quoted 11 lines

src/smsutil.c void status_report_assembly_expire(struct status_report_assembly *assembly, · line 3150

Quoted 8 lines
src/smsutil.c +8 -0
+	/*
+	 * file name is: imsi/order in tx_queue-uuid/order of pdus
+	 */
+	if (write_file(buf, len, SMS_BACKUP_MODE,
+				SMS_TX_BACKUP_PATH_FILE, imsi, id, uuid,
+				seq) != len)
+		return FALSE;
+

And where are you taking care of storing the other needed information? e.g. the tx_queue entry flags, retry count, etc?

Quoted 5 lines

I can easily add the flags and retry count. As far as the "etc" that you mention, the only other information I see would be entry->cb and entry->data, which I was thinking might have to get lost. I am not sure how to persist that information on disk - do you have any suggestions?

There's really no way to persist these, so losing them is fine for now.
The only entity who makes use of the callback is the stk atom, and
those sms messages should not be serialized to disk. e.g. we should not try to persist messages that do not have at least the OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS set.

Regards,
-Denis

author
#5 lore raw

PATCH v2 1/3 sms: store pending tx pdus on disk

In reply to Denis Kenzior
---
 src/sms.c     |   15 +++++++++++++++
 src/smsutil.c |   26 ++++++++++++++++++++++++++
 src/smsutil.h |    3 +++
 3 files changed, 44 insertions(+), 0 deletions(-)
src/sms.c +15 -0
index 163eab0..e5e06db 100644
@@ -77,6 +77,7 @@ struct ofono_sms {
 	struct sms_assembly *assembly;
 	guint ref;
 	GQueue *txq;
+	unsigned long tx_counter;
 	guint tx_source;
 	struct ofono_message_waiting *mw;
 	unsigned int mw_watch;

@@ -111,6 +112,7 @@ struct tx_queue_entry {
 	ofono_sms_txq_submit_cb_t cb;
 	void *data;
 	ofono_destroy_func destroy;
+	unsigned long id;
 };
 
 static gboolean uuid_equal(gconstpointer v1, gconstpointer v2)

@@ -1896,6 +1898,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 {
 	struct message *m = NULL;
 	struct tx_queue_entry *entry;
+	int i;
+	GSList *l;
 
 	entry = tx_queue_entry_new(list, flags);
 	if (entry == NULL)

@@ -1920,6 +1924,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 			sms->ref = sms->ref + 1;
 	}
 
+	entry->id = sms->tx_counter++;
+
 	g_queue_push_tail(sms->txq, entry);
 
 	if (g_queue_get_length(sms->txq) == 1)

@@ -1928,6 +1934,15 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 	if (uuid)
 		memcpy(uuid, &entry->uuid, sizeof(*uuid));
 
+	if (flags & OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS) {
+		for (i = 0, l = list; l; i++, l = l->next) {
+			struct sms *s = l->data;
+			sms_tx_store(sms->imsi, entry->id, entry->flags,
+					ofono_uuid_to_str(&entry->uuid),
+					s, i);
+		}
+	}
+
 	if (cb)
 		cb(sms, &entry->uuid, data);
src/smsutil.c +26 -0
index 5e1d233..9c6596f 100644
@@ -48,6 +48,10 @@
 #define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
 #define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%s"
 
+#define SMS_TX_BACKUP_PATH STORAGEDIR "/%s/tx_queue"
+#define SMS_TX_BACKUP_PATH_DIR SMS_TX_BACKUP_PATH "/%lu-%lu-%s"
+#define SMS_TX_BACKUP_PATH_FILE SMS_TX_BACKUP_PATH_DIR "/%03i"
+
 #define SMS_ADDR_FMT "%24[0-9A-F]"
 #define SMS_MSGID_FMT "%40[0-9A-F]"
 

@@ -3143,6 +3147,28 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
+			const char *uuid, struct sms *s, guint8 seq)
+{
+	int len;
+	unsigned char buf[177];
+
+	if (!imsi)
+		return FALSE;
+
+	len = sms_serialize(buf, s);
+
+	/*
+	 * file name is: imsi/order in tx_queue-flags-uuid/order of pdus
+	 */
+	if (write_file(buf, len, SMS_BACKUP_MODE,
+				SMS_TX_BACKUP_PATH_FILE, imsi, id, flags, uuid,
+				seq) != len)
+		return FALSE;
+
+	return TRUE;
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
src/smsutil.h +3 -0
index 4b6159f..46084ab 100644
@@ -517,6 +517,9 @@ void status_report_assembly_add_fragment(struct status_report_assembly
 void status_report_assembly_expire(struct status_report_assembly *assembly,
 					time_t before);
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
+				const char *uuid, struct sms *s, guint8 seq);
+
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
 				gboolean use_delivery_reports);
Signature · 1 line
1.7.2.3
#6 lore raw

Re: [PATCH 1/3] sms: store pending tx pdus on disk

In reply to Kristen Carlson Accardi

Hi Kristen,

2010/11/25 Kristen Carlson Accardi <[email protected]>:

src/sms.c int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, · line 1933

Quoted 7 lines
src/sms.c +2 -0
@@ -1923,6 +1928,8 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
                        sms->ref = sms->ref + 1;
        }
 
+       entry->id = tx_counter++;
+
        g_queue_push_tail(sms->txq, entry);

Can't you use g_queue_get_length() here to get the position of the newly pushed entry? That would make tx_counter unnecessary (and anyway, it should probably be internal to each sms atom data instead of a static variable).

Quoted 34 lines
src/sms.c +10 -0
        if (g_queue_get_length(sms->txq) == 1)

@@ -1931,6 +1938,16 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
        if (uuid)
                memcpy(uuid, &entry->uuid, sizeof(*uuid));
 
+       i = 0;
+
+       for (l = list; l; l = l->next) {
+               struct sms *s = l->data;
+               sms_tx_store(sms->imsi, entry->id,
+                               ofono_uuid_to_str(&entry->uuid),
+                               s, i);
+               i++;
+       }
+
        if (cb)
                cb(sms, &entry->uuid, data);
src/smsutil.c +4 -0
index e6dbf5f..4904419 100644
@@ -48,6 +48,10 @@
  #define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
  #define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH "/%s-%s"
 
+#define SMS_TX_BACKUP_PATH STORAGEDIR "/%s/tx_queue"
+#define SMS_TX_BACKUP_PATH_DIR SMS_TX_BACKUP_PATH "/%lu-%s"
+#define SMS_TX_BACKUP_PATH_FILE SMS_TX_BACKUP_PATH_DIR "/%03i"
+
  #define SMS_ADDR_FMT "%24[0-9A-F]"
  #define SMS_MSGID_FMT "%40[0-9A-F]"

src/smsutil.c void status_report_assembly_expire(struct status_report_assembly *assembly, · line 3138

Quoted 10 lines
src/smsutil.c +7 -0
@@ -3125,6 +3129,28 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
        }
  }
 
+gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
+                       struct sms *s, guint8 seq)
+{
+       int len;
+       unsigned char buf[177];
+
+       if (!imsi)

Need to explicitly compare to NULL here. A recently added convention, I admit.

Cheers,
Aki

author
#7 lore raw

PATCH 2/3 sms: delete sent sms messages from backup

In reply to Kristen Carlson Accardi
---
 src/sms.c     |    9 +++++++++
 src/smsutil.c |   38 ++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    4 ++++
 3 files changed, 51 insertions(+), 0 deletions(-)
src/sms.c +9 -0
index 6eab0fa..f987946 100644
@@ -769,6 +769,10 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 		goto next_q;
 	}
 
+	sms_tx_backup_remove(sms->imsi, entry->id,
+				ofono_uuid_to_str(&entry->uuid),
+				entry->cur_pdu);
+
 	entry->cur_pdu += 1;
 	entry->retry = 0;
 

@@ -813,8 +817,13 @@ next_q:
 		message_set_state(sms, &entry->uuid, ms);
 	}
 
+	sms_tx_backup_destroy(sms->imsi, entry->id,
+			ofono_uuid_to_str(&entry->uuid));
+
 	tx_queue_entry_destroy(entry);
 
+	tx_counter--;
+
 	if (g_queue_peek_head(sms->txq)) {
 		DBG("Scheduling next");
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
src/smsutil.c +38 -0
index 4904419..7a6b70a 100644
@@ -3151,6 +3151,44 @@ gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
 	return TRUE;
 }
 
+void sms_tx_backup_destroy(const char *imsi, unsigned long id, const char *uuid)
+{
+	char *path;
+	struct dirent **entries;
+	int len;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_DIR,
+					imsi, id, uuid);
+
+	len = scandir(path, &entries, NULL, versionsort);
+	while (len--) {
+		struct dirent *dir = entries[len];
+		char *file = g_strdup_printf("%s/%s", path, dir->d_name);
+
+		unlink(file);
+
+		g_free(file);
+		g_free(entries[len]);
+	}
+	g_free(entries);
+
+	rmdir(path);
+
+	g_free(path);
+}
+
+void sms_tx_backup_remove(const char *imsi, unsigned long id, const char *uuid,
+				guint8 seq)
+{
+	char *path;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_FILE,
+					imsi, id, uuid, seq);
+	unlink(path);
+
+	g_free(path);
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
src/smsutil.h +4 -0
index 5279489..2e2bae6 100644
@@ -518,6 +518,10 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 
 gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
 				struct sms *s, guint8 seq);
+void sms_tx_backup_remove(const char *imsi, unsigned long id, const char *uuid,
+				guint8 seq);
+void sms_tx_backup_destroy(const char *imsi, unsigned long id,
+				const char *uuid);
 
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
Signature · 1 line
1.7.2.3
#8 lore raw

Re: [PATCH 2/3] sms: delete sent sms messages from backup

Hi Kristen,

Quoted 22 lines · Kristen Carlson Accardi
---
 src/sms.c     |    9 +++++++++
 src/smsutil.c |   38 ++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    4 ++++
 3 files changed, 51 insertions(+), 0 deletions(-)
src/sms.c +4 -0
index 6eab0fa..f987946 100644
@@ -769,6 +769,10 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 		goto next_q;
 	}
 
+	sms_tx_backup_remove(sms->imsi, entry->id,
+				ofono_uuid_to_str(&entry->uuid),
+				entry->cur_pdu);
+
 	entry->cur_pdu += 1;
 	entry->retry = 0;

src/sms.c next_q: · line 826

Quoted 10 lines
src/sms.c +5 -0
@@ -813,8 +817,13 @@ next_q:
 		message_set_state(sms, &entry->uuid, ms);
 	}
 
+	sms_tx_backup_destroy(sms->imsi, entry->id,
+			ofono_uuid_to_str(&entry->uuid));
+
 	tx_queue_entry_destroy(entry);
 
+	tx_counter--;
+

I'm not sure this is going to work, e.g. start with tx_counter == 0:

Queue -> tx_counter = 1, id = 0
Queue -> tx_counter = 2, id = 1,
Queue -> tx_counter = 3, id = 2,

Sent -> tx_counter = 2, (id of 0 sent)

Queue -> tx_counter = 3, id = 2

Now you have two messages with the same id and lost all guarantees on the order...

Perhaps simply relying on always increasing tx_counter might be enough.
It is unlikely anyone will ever send 4 billion SMSes ;) Renaming the
directory during the restore operation might be an option as well.

src/smsutil.c gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid, · line 3154

Quoted 12 lines
src/sms.c +0 -0
 	if (g_queue_peek_head(sms->txq)) {
 		DBG("Scheduling next");
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
src/smsutil.c +1 -0
index 4904419..7a6b70a 100644
@@ -3151,6 +3151,44 @@ gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
 	return TRUE;
 }
 
+void sms_tx_backup_destroy(const char *imsi, unsigned long id, const char *uuid)

The existing code tends to name this foo_backup_free rather than backup_destroy.

src/smsutil.c gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid, · line 3163

Quoted 9 lines
src/smsutil.c +9 -0
+{
+	char *path;
+	struct dirent **entries;
+	int len;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_DIR,
+					imsi, id, uuid);
+
+	len = scandir(path, &entries, NULL, versionsort);

Rule M1 please

src/smsutil.c gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid, · line 3172

Quoted 9 lines
src/smsutil.c +9 -0
+	while (len--) {
+		struct dirent *dir = entries[len];
+		char *file = g_strdup_printf("%s/%s", path, dir->d_name);
+
+		unlink(file);
+
+		g_free(file);
+		g_free(entries[len]);
+	}

Rule M1 please

Quoted 27 lines
src/smsutil.c +19 -0
+	g_free(entries);
+
+	rmdir(path);
+
+	g_free(path);
+}
+
+void sms_tx_backup_remove(const char *imsi, unsigned long id, const char *uuid,
+				guint8 seq)
+{
+	char *path;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_FILE,
+					imsi, id, uuid, seq);
+	unlink(path);
+
+	g_free(path);
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
src/smsutil.h +0 -0
index 5279489..2e2bae6 100644

src/smsutil.h void status_report_assembly_expire(struct status_report_assembly *assembly, · line 527

Quoted 10 lines
src/smsutil.h +4 -0
@@ -518,6 +518,10 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 
 gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
 				struct sms *s, guint8 seq);
+void sms_tx_backup_remove(const char *imsi, unsigned long id, const char *uuid,
+				guint8 seq);
+void sms_tx_backup_destroy(const char *imsi, unsigned long id,
+				const char *uuid);
 
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,

Regards,
-Denis

author
#9 lore raw

PATCH v2 2/3 sms: delete sent sms messages from backup

---
 src/sms.c     |    7 +++++++
 src/smsutil.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    5 +++++
 3 files changed, 57 insertions(+), 0 deletions(-)
src/sms.c +7 -0
index e5e06db..dd4d27c 100644
@@ -766,6 +766,10 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 		goto next_q;
 	}
 
+	sms_tx_backup_remove(sms->imsi, entry->id, entry->flags,
+				ofono_uuid_to_str(&entry->uuid),
+				entry->cur_pdu);
+
 	entry->cur_pdu += 1;
 	entry->retry = 0;
 

@@ -810,6 +814,9 @@ next_q:
 		message_set_state(sms, &entry->uuid, ms);
 	}
 
+	sms_tx_backup_free(sms->imsi, entry->id, entry->flags,
+			ofono_uuid_to_str(&entry->uuid));
+
 	tx_queue_entry_destroy(entry);
 
 	if (g_queue_peek_head(sms->txq)) {
src/smsutil.c +45 -0

51 lines not on this page · raw message

src/smsutil.h +5 -0
index 46084ab..fa4e453 100644
@@ -519,6 +519,11 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 
 gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
 				const char *uuid, struct sms *s, guint8 seq);
+void sms_tx_backup_remove(const char *imsi, unsigned long id,
+				unsigned long flags, const char *uuid,
+				guint8 seq);
+void sms_tx_backup_free(const char *imsi, unsigned long id,
+				unsigned long flags, const char *uuid);
 
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
Signature · 1 line
1.7.2.3
author
#10 lore raw

PATCH 3/3 sms: restore pending tx messages from backup

In reply to Kristen Carlson Accardi
---
 src/ofono.h   |    1 +
 src/sms.c     |   38 +++++++++++++++++++
 src/smsutil.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    6 +++
 4 files changed, 161 insertions(+), 0 deletions(-)
src/ofono.h +1 -0
index d1a4bdc..4777f70 100644
@@ -241,6 +241,7 @@ enum ofono_sms_submit_flag {
 	OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY =	0x2,
 	OFONO_SMS_SUBMIT_FLAG_RETRY =		0x4,
 	OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS =	0x8,
+	OFONO_SMS_SUBMIT_FLAG_REUSE_UUID =	0x10,
 };
 
 typedef void (*ofono_sms_txq_submit_cb_t)(gboolean ok, void *data);
src/sms.c +38 -0
index f987946..be710b1 100644
@@ -1814,6 +1814,40 @@ static void bearer_init_callback(const struct ofono_error *error, void *data)
 		ofono_error("Error bootstrapping SMS Bearer Preference");
 }
 
+static void sms_restore_tx_queue(struct ofono_sms *sms)
+{
+	GQueue *backupq;
+	unsigned int flags;
+	struct txq_backup_entry *entry;
+	struct ofono_uuid ofono_uuid;
+
+	DBG("");
+
+	flags = OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY;
+	flags |= OFONO_SMS_SUBMIT_FLAG_RETRY;
+	flags |= OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+
+	if (sms->use_delivery_reports)
+		flags |= OFONO_SMS_SUBMIT_FLAG_REQUEST_SR;
+
+	backupq = sms_tx_queue_load(sms->imsi);
+
+	while ((entry = g_queue_pop_head(backupq))) {
+		decode_hex_own_buf(entry->uuid, -1, NULL, 0, ofono_uuid.uuid);
+
+		__ofono_sms_txq_submit(sms, entry->msg_list, flags,
+					&ofono_uuid, NULL, NULL);
+
+		g_slist_foreach(entry->msg_list, (GFunc)g_free, NULL);
+		g_slist_free(entry->msg_list);
+
+		g_free(entry->uuid);
+		g_free(entry);
+	}
+
+	g_queue_free(backupq);
+}
+
 /*
  * Indicate oFono that a SMS driver is ready for operation
  *

@@ -1877,6 +1911,7 @@ void ofono_sms_register(struct ofono_sms *sms)
 	if (sms->driver->bearer_set)
 		sms->driver->bearer_set(sms, sms->bearer,
 						bearer_init_callback, sms);
+	sms_restore_tx_queue(sms);
 
 	sms->text_handlers = __ofono_watchlist_new(g_free);
 	sms->datagram_handlers = __ofono_watchlist_new(g_free);

@@ -1918,6 +1953,9 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 	if (entry == NULL)
 		return -ENOMEM;
 
+	if (flags & OFONO_SMS_SUBMIT_FLAG_REUSE_UUID)
+		memcpy(&entry->uuid, uuid, sizeof(*uuid));
+
 	if (flags & OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS) {
 		m = message_create(&entry->uuid);
 		if (m == NULL)
src/smsutil.c +116 -0

128 lines not on this page · raw message

src/smsutil.h +6 -0
index 2e2bae6..8d579a7 100644
@@ -407,6 +407,11 @@ struct cbs_topic_range {
 	unsigned short max;
 };
 
+struct txq_backup_entry {
+	GSList *msg_list;
+	char *uuid;
+};
+
 static inline gboolean is_bit_set(unsigned char oct, int bit)
 {
 	int mask = 0x1 << bit;

@@ -522,6 +527,7 @@ void sms_tx_backup_remove(const char *imsi, unsigned long id, const char *uuid,
 				guint8 seq);
 void sms_tx_backup_destroy(const char *imsi, unsigned long id,
 				const char *uuid);
+GQueue *sms_tx_queue_load(const char *imsi);
 
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
Signature · 1 line
1.7.2.3
#11 lore raw

Re: [PATCH 3/3] sms: restore pending tx messages from backup

Hi Kristen,

Quoted 32 lines · Kristen Carlson Accardi
---
 src/ofono.h   |    1 +
 src/sms.c     |   38 +++++++++++++++++++
 src/smsutil.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    6 +++
 4 files changed, 161 insertions(+), 0 deletions(-)
src/ofono.h +1 -0
index d1a4bdc..4777f70 100644
@@ -241,6 +241,7 @@ enum ofono_sms_submit_flag {
 	OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY =	0x2,
 	OFONO_SMS_SUBMIT_FLAG_RETRY =		0x4,
 	OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS =	0x8,
+	OFONO_SMS_SUBMIT_FLAG_REUSE_UUID =	0x10,
 };
 
 typedef void (*ofono_sms_txq_submit_cb_t)(gboolean ok, void *data);
src/sms.c +5 -0
index f987946..be710b1 100644
@@ -1814,6 +1814,40 @@ static void bearer_init_callback(const struct ofono_error *error, void *data)
 		ofono_error("Error bootstrapping SMS Bearer Preference");
 }
 
+static void sms_restore_tx_queue(struct ofono_sms *sms)
+{
+	GQueue *backupq;
+	unsigned int flags;
+	struct txq_backup_entry *entry;

src/sms.c static void bearer_init_callback(const struct ofono_error *error, void *data) · line 1831

Quoted 10 lines
src/sms.c +10 -0
+	struct ofono_uuid ofono_uuid;
+
+	DBG("");
+
+	flags = OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY;
+	flags |= OFONO_SMS_SUBMIT_FLAG_RETRY;
+	flags |= OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+
+	if (sms->use_delivery_reports)
+		flags |= OFONO_SMS_SUBMIT_FLAG_REQUEST_SR;

As I mentioned before, you should be restoring this information. Assuming that it is the same for all messages on the tx queue is a really bad idea.

src/sms.c static void bearer_init_callback(const struct ofono_error *error, void *data) · line 1839

Quoted 8 lines
src/sms.c +8 -0
+
+	backupq = sms_tx_queue_load(sms->imsi);
+
+	while ((entry = g_queue_pop_head(backupq))) {
+		decode_hex_own_buf(entry->uuid, -1, NULL, 0, ofono_uuid.uuid);
+
+		__ofono_sms_txq_submit(sms, entry->msg_list, flags,
+					&ofono_uuid, NULL, NULL);

Using sms_txq_submit is a bad idea. This one will re-calculate the UUID which gets tossed out later. I suggest creating the tx_queue entries 'raw'.

Quoted 4 lines
src/sms.c +4 -0
+
+		g_slist_foreach(entry->msg_list, (GFunc)g_free, NULL);
+		g_slist_free(entry->msg_list);
+

src/sms.c static void bearer_init_callback(const struct ofono_error *error, void *data) · line 1853

Quoted 10 lines
src/sms.c +7 -0
+		g_free(entry->uuid);
+		g_free(entry);
+	}
+
+	g_queue_free(backupq);
+}
+
 /*
  * Indicate oFono that a SMS driver is ready for operation
  *

<snip>

Regards,
-Denis

author
#12 lore raw

PATCH v2 3/3 sms: restore pending tx messages from backup

---
 src/ofono.h   |    1 +
 src/sms.c     |   75 ++++++++++++++++++++++++++++++
 src/smsutil.c |  143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    8 +++
 4 files changed, 227 insertions(+), 0 deletions(-)
src/ofono.h +1 -0
index 792134b..c8e6cf0 100644
@@ -243,6 +243,7 @@ enum ofono_sms_submit_flag {
 	OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY =	0x2,
 	OFONO_SMS_SUBMIT_FLAG_RETRY =		0x4,
 	OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS =	0x8,
+	OFONO_SMS_SUBMIT_FLAG_REUSE_UUID =	0x10,
 };
 
 typedef void (*ofono_sms_txq_submit_cb_t)(gboolean ok, void *data);
src/sms.c +75 -0

93 lines not on this page · raw message

src/smsutil.c +143 -0

155 lines not on this page · raw message

src/smsutil.h +8 -0
index fa4e453..38d8dc9 100644
@@ -408,6 +408,13 @@ struct cbs_topic_range {
 	unsigned short max;
 };
 
+struct txq_backup_entry {
+	GSList *msg_list;
+	char *uuid;
+	unsigned long flags;
+	char *oldpath;
+};
+
 static inline gboolean is_bit_set(unsigned char oct, int bit)
 {
 	int mask = 0x1 << bit;

@@ -524,6 +531,7 @@ void sms_tx_backup_remove(const char *imsi, unsigned long id,
 				guint8 seq);
 void sms_tx_backup_free(const char *imsi, unsigned long id,
 				unsigned long flags, const char *uuid);
+GQueue *sms_tx_queue_load(const char *imsi);
 
 GSList *sms_text_prepare(const char *to, const char *utf8, guint16 ref,
 				gboolean use_16bit,
Signature · 1 line
1.7.2.3