[PATCH 0/3] Persist TX SMS messages

[PATCH 0/3] Persist TX SMS messages

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]

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(-)

-- 
1.7.2.3

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 3346 bytes --]

---
 src/sms.c     |   17 +++++++++++++++++
 src/smsutil.c |   26 ++++++++++++++++++++++++++
 src/smsutil.h |    3 +++
 3 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index 12988c8..6eab0fa 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -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);
 
diff --git a/src/smsutil.c b/src/smsutil.c
index e6dbf5f..4904419 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -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;
diff --git a/src/smsutil.h b/src/smsutil.h
index 4b05313..5279489 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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);
-- 
1.7.2.3

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

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 4105 bytes --]

Hi Kristen,

On 11/24/2010 04:53 PM, Kristen Carlson Accardi wrote:
> ---
>  src/sms.c     |   17 +++++++++++++++++
>  src/smsutil.c |   26 ++++++++++++++++++++++++++
>  src/smsutil.h |    3 +++
>  3 files changed, 46 insertions(+), 0 deletions(-)
> 
> diff --git a/src/sms.c b/src/sms.c
> index 12988c8..6eab0fa 100644
> --- a/src/sms.c
> +++ b/src/sms.c
> @@ -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.

>  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?

>  };
>  
>  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) {

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

> +		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);
>  
> diff --git a/src/smsutil.c b/src/smsutil.c
> index e6dbf5f..4904419 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -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;
> +

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

> +	return TRUE;
> +}
> +
>  static inline GSList *sms_list_append(GSList *l, const struct sms *in)
>  {
>  	struct sms *sms;
> diff --git a/src/smsutil.h b/src/smsutil.h
> index 4b05313..5279489 100644
> --- a/src/smsutil.h
> +++ b/src/smsutil.h
> @@ -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

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On Mon, 06 Dec 2010 20:38:58 -0600
Denis Kenzior <denkenz@gmail.com> wrote:

<snip>

> > +	/*
> > +	 * 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?

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

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]

On 12/07/2010 04:57 PM, Kristen Carlson Accardi wrote:
> On Mon, 06 Dec 2010 20:38:58 -0600
> Denis Kenzior <denkenz@gmail.com> wrote:
> 
> <snip>
> 
>>> +	/*
>>> +	 * 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?

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

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 3475 bytes --]

---
 src/sms.c     |   15 +++++++++++++++
 src/smsutil.c |   26 ++++++++++++++++++++++++++
 src/smsutil.h |    3 +++
 3 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index 163eab0..e5e06db 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -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);
 
diff --git a/src/smsutil.c b/src/smsutil.c
index 5e1d233..9c6596f 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -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;
diff --git a/src/smsutil.h b/src/smsutil.h
index 4b6159f..46084ab 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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);
-- 
1.7.2.3

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

[Aki Niemi]

[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]

Hi Kristen,

2010/11/25 Kristen Carlson Accardi <kristen@linux.intel.com>:
> @@ -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).

>        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);
>
> diff --git a/src/smsutil.c b/src/smsutil.c
> index e6dbf5f..4904419 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -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)

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

Cheers,
Aki

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 2672 bytes --]

---
 src/sms.c     |    9 +++++++++
 src/smsutil.c |   38 ++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    4 ++++
 3 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index 6eab0fa..f987946 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -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);
diff --git a/src/smsutil.c b/src/smsutil.c
index 4904419..7a6b70a 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -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;
diff --git a/src/smsutil.h b/src/smsutil.h
index 5279489..2e2bae6 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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,
-- 
1.7.2.3

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

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 3628 bytes --]

Hi Kristen,

On 11/24/2010 04:53 PM, Kristen Carlson Accardi wrote:
> ---
>  src/sms.c     |    9 +++++++++
>  src/smsutil.c |   38 ++++++++++++++++++++++++++++++++++++++
>  src/smsutil.h |    4 ++++
>  3 files changed, 51 insertions(+), 0 deletions(-)
> 
> diff --git a/src/sms.c b/src/sms.c
> index 6eab0fa..f987946 100644
> --- a/src/sms.c
> +++ b/src/sms.c
> @@ -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--;
> +

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.

>  	if (g_queue_peek_head(sms->txq)) {
>  		DBG("Scheduling next");
>  		sms->tx_source = g_timeout_add(0, tx_next, sms);
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 4904419..7a6b70a 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -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.

> +{
> +	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

> +	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

> +	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;
> diff --git a/src/smsutil.h b/src/smsutil.h
> index 5279489..2e2bae6 100644
> --- a/src/smsutil.h
> +++ b/src/smsutil.h
> @@ -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

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 2772 bytes --]

---
 src/sms.c     |    7 +++++++
 src/smsutil.c |   45 +++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    5 +++++
 3 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index e5e06db..dd4d27c 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -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)) {
diff --git a/src/smsutil.c b/src/smsutil.c
index 9c6596f..09da657 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -3169,6 +3169,51 @@ gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
 	return TRUE;
 }
 
+void sms_tx_backup_free(const char *imsi, unsigned long id,
+				unsigned long flags, const char *uuid)
+{
+	char *path;
+	struct dirent **entries;
+	int len;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_DIR,
+					imsi, id, flags, uuid);
+
+	len = scandir(path, &entries, NULL, versionsort);
+
+	if (len < 0)
+		return;
+
+	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,
+				unsigned long flags, const char *uuid,
+				guint8 seq)
+{
+	char *path;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH_FILE,
+					imsi, id, flags, uuid, seq);
+	unlink(path);
+
+	g_free(path);
+}
+
 static inline GSList *sms_list_append(GSList *l, const struct sms *in)
 {
 	struct sms *sms;
diff --git a/src/smsutil.h b/src/smsutil.h
index 46084ab..fa4e453 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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,
-- 
1.7.2.3

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 6200 bytes --]

---
 src/ofono.h   |    1 +
 src/sms.c     |   38 +++++++++++++++++++
 src/smsutil.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    6 +++
 4 files changed, 161 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index d1a4bdc..4777f70 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -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);
diff --git a/src/sms.c b/src/sms.c
index f987946..be710b1 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -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)
diff --git a/src/smsutil.c b/src/smsutil.c
index 7a6b70a..188b96e 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2309,6 +2309,15 @@ static gboolean sms_deserialize(const unsigned char *buf,
 	return sms_decode(buf + 1, len - 1, FALSE, buf[0], sms);
 }
 
+static gboolean sms_deserialize_outgoing(const unsigned char *buf,
+		struct sms *sms, int len)
+{
+	if (len < 1)
+		return FALSE;
+
+	return sms_decode(buf + 1, len - 1, TRUE, buf[0], sms);
+}
+
 static gboolean sms_assembly_extract_address(const char *straddr,
 						struct sms_address *out)
 {
@@ -3129,6 +3138,113 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+/*
+ * Each directory contains a file per pdu.
+ */
+static GSList *sms_tx_load(const char *imsi, const struct dirent *dir)
+{
+	GSList *list = NULL;
+	struct dirent **pdus;
+	char *path;
+	int len, i, r;
+	unsigned char buf[177];
+	struct sms s;
+
+	if (dir->d_type != DT_DIR)
+		return NULL;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH "/%s",
+			imsi, dir->d_name);
+
+	len = scandir(path, &pdus, NULL, versionsort);
+	g_free(path);
+
+	if (len < 0)
+		return NULL;
+
+	for (i = 0; i < len; i++) {
+		if (pdus[i]->d_type != DT_REG)
+			continue;
+
+		if ((strcmp(dir->d_name, ".") == 0) ||
+				(strcmp(dir->d_name, "..") == 0))
+			continue;
+
+		r = read_file(buf, sizeof(buf), SMS_TX_BACKUP_PATH "/%s/%s",
+				imsi, dir->d_name, pdus[i]->d_name);
+		if (r < 0)
+			continue;
+
+		if (sms_deserialize_outgoing(buf, &s, r) == FALSE)
+			continue;
+
+		list = g_slist_prepend(list, g_memdup(&s, sizeof(s)));
+	}
+
+	for (i = 0; i < len; i++)
+		g_free(pdus[i]);
+
+	g_free(pdus);
+
+	return g_slist_reverse(list);
+}
+
+/*
+ * populate the queue with tx_backup_entry from stored backup
+ * data.
+ */
+GQueue *sms_tx_queue_load(const char *imsi)
+{
+	char *path;
+	struct dirent **entries;
+	int len;
+	GSList *msg_list;
+	char *uuid;
+	GQueue *retq;
+
+	retq = g_queue_new();
+	if (retq == NULL)
+		return NULL;
+
+	if (imsi) {
+		/* Restore state from backup */
+		path = g_strdup_printf(SMS_TX_BACKUP_PATH, imsi);
+		len = scandir(path, &entries, NULL, versionsort);
+		g_free(path);
+
+		if (len < 0)
+			return retq;
+
+		while (len--) {
+			struct txq_backup_entry *entry;
+
+			struct dirent *dir = entries[len];
+
+			msg_list = sms_tx_load(imsi, dir);
+
+			sscanf(dir->d_name, "%*u-%as", &uuid);
+
+			g_free(entries[len]);
+
+			if (g_slist_length(msg_list) == 0)
+				continue;
+
+			entry = g_try_new0(struct txq_backup_entry, 1);
+			if (entry == NULL)
+				continue;
+
+			entry->msg_list = msg_list;
+			entry->uuid = uuid;
+
+			g_queue_push_head(retq, entry);
+		}
+
+		g_free(entries);
+	}
+
+	return retq;
+}
+
 gboolean sms_tx_store(const char *imsi, unsigned long id, const char *uuid,
 			struct sms *s, guint8 seq)
 {
diff --git a/src/smsutil.h b/src/smsutil.h
index 2e2bae6..8d579a7 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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,
-- 
1.7.2.3

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

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]

Hi Kristen,

On 11/24/2010 04:53 PM, Kristen Carlson Accardi wrote:
> ---
>  src/ofono.h   |    1 +
>  src/sms.c     |   38 +++++++++++++++++++
>  src/smsutil.c |  116 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/smsutil.h |    6 +++
>  4 files changed, 161 insertions(+), 0 deletions(-)
> 
> diff --git a/src/ofono.h b/src/ofono.h
> index d1a4bdc..4777f70 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -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);
> diff --git a/src/sms.c b/src/sms.c
> index f987946..be710b1 100644
> --- a/src/sms.c
> +++ b/src/sms.c
> @@ -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;

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.

> +
> +	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'.

> +
> +		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
>   *

<snip>

Regards,
-Denis

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

[Kristen Carlson Accardi]

[-- Attachment #1: Type: text/plain, Size: 7870 bytes --]

---
 src/ofono.h   |    1 +
 src/sms.c     |   75 ++++++++++++++++++++++++++++++
 src/smsutil.c |  143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    8 +++
 4 files changed, 227 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index 792134b..c8e6cf0 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -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);
diff --git a/src/sms.c b/src/sms.c
index dd4d27c..42ca034 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -929,6 +929,9 @@ static struct tx_queue_entry *tx_queue_entry_new(GSList *msg_list,
 				pdu->pdu_len, pdu->tpdu_len);
 	}
 
+	if (flags & OFONO_SMS_SUBMIT_FLAG_REUSE_UUID)
+		return entry;
+
 	if (sms_uuid_from_pdus(entry->pdus, entry->num_pdus, &entry->uuid))
 		return entry;
 
@@ -1808,6 +1811,77 @@ 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;
+	struct txq_backup_entry *backup_entry;
+	struct ofono_uuid ofono_uuid;
+
+	DBG("");
+
+	backupq = sms_tx_queue_load(sms->imsi);
+
+	while ((backup_entry = g_queue_pop_head(backupq))) {
+		struct message *m;
+		struct tx_queue_entry *txq_entry;
+
+		decode_hex_own_buf(backup_entry->uuid, -1, NULL, 0,
+					ofono_uuid.uuid);
+
+		backup_entry->flags |= OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+
+		txq_entry = tx_queue_entry_new(backup_entry->msg_list,
+						backup_entry->flags);
+
+		if (txq_entry == NULL)
+			goto loop_out;
+
+		txq_entry->flags &= ~OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+
+		memcpy(&txq_entry->uuid, &ofono_uuid, sizeof(ofono_uuid));
+
+		m = message_create(&txq_entry->uuid);
+
+		if (m == NULL) {
+			tx_queue_entry_destroy(txq_entry);
+			goto loop_out;
+		}
+
+		if (message_dbus_register(sms, m) == FALSE) {
+			tx_queue_entry_destroy(txq_entry);
+			goto loop_out;
+		}
+
+		g_hash_table_insert(sms->messages, &m->uuid, m);
+		m->entry = txq_entry;
+
+		if (backup_entry->msg_list->next != NULL) {
+			if (sms->ref == 65536)
+				sms->ref = 1;
+			else
+				sms->ref = sms->ref + 1;
+		}
+
+		txq_entry->id = sms->tx_counter++;
+
+		g_queue_push_tail(sms->txq, txq_entry);
+
+		if (g_queue_get_length(sms->txq) == 1)
+			sms->tx_source = g_timeout_add(0, tx_next, sms);
+
+		if (backup_entry->flags & OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS)
+			emit_message_added(sms, m);
+loop_out:
+		g_slist_foreach(backup_entry->msg_list, (GFunc)g_free, NULL);
+		g_slist_free(backup_entry->msg_list);
+
+		g_free(backup_entry->uuid);
+		g_free(backup_entry);
+	}
+
+	g_queue_free(backupq);
+}
+
 /*
  * Indicate oFono that a SMS driver is ready for operation
  *
@@ -1871,6 +1945,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);
diff --git a/src/smsutil.c b/src/smsutil.c
index 09da657..3d33a54 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2327,6 +2327,15 @@ static gboolean sms_deserialize(const unsigned char *buf,
 	return sms_decode(buf + 1, len - 1, FALSE, buf[0], sms);
 }
 
+static gboolean sms_deserialize_outgoing(const unsigned char *buf,
+		struct sms *sms, int len)
+{
+	if (len < 1)
+		return FALSE;
+
+	return sms_decode(buf + 1, len - 1, TRUE, buf[0], sms);
+}
+
 static gboolean sms_assembly_extract_address(const char *straddr,
 						struct sms_address *out)
 {
@@ -3147,6 +3156,140 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+/*
+ * Each directory contains a file per pdu.
+ */
+static GSList *sms_tx_load(const char *imsi, const struct dirent *dir)
+{
+	GSList *list = NULL;
+	struct dirent **pdus;
+	char *path;
+	int len, i, r;
+	unsigned char buf[177];
+	struct sms s;
+
+	if (dir->d_type != DT_DIR)
+		return NULL;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH "/%s",
+			imsi, dir->d_name);
+
+	len = scandir(path, &pdus, NULL, versionsort);
+	g_free(path);
+
+	if (len < 0)
+		return NULL;
+
+	for (i = 0; i < len; i++) {
+		if (pdus[i]->d_type != DT_REG)
+			continue;
+
+		if ((strcmp(dir->d_name, ".") == 0) ||
+				(strcmp(dir->d_name, "..") == 0))
+			continue;
+
+		r = read_file(buf, sizeof(buf), SMS_TX_BACKUP_PATH "/%s/%s",
+				imsi, dir->d_name, pdus[i]->d_name);
+		if (r < 0)
+			continue;
+
+		if (sms_deserialize_outgoing(buf, &s, r) == FALSE)
+			continue;
+
+		list = g_slist_prepend(list, g_memdup(&s, sizeof(s)));
+	}
+
+	for (i = 0; i < len; i++)
+		g_free(pdus[i]);
+
+	g_free(pdus);
+
+	return g_slist_reverse(list);
+}
+
+/*
+ * populate the queue with tx_backup_entry from stored backup
+ * data.
+ */
+GQueue *sms_tx_queue_load(const char *imsi)
+{
+	char *path;
+	struct dirent **entries;
+	int len;
+	GSList *msg_list;
+	char *uuid;
+	GQueue *retq;
+	unsigned long flags;
+	unsigned long i;
+
+	retq = g_queue_new();
+	if (retq == NULL)
+		return NULL;
+
+	if (imsi) {
+		/* Restore state from backup */
+		path = g_strdup_printf(SMS_TX_BACKUP_PATH, imsi);
+		len = scandir(path, &entries, NULL, versionsort);
+
+		if (len < 0) {
+			g_free(path);
+			return retq;
+		}
+
+		while (len--) {
+			struct txq_backup_entry *entry;
+			char *oldpath;
+			struct dirent *dir = entries[len];
+
+			msg_list = sms_tx_load(imsi, dir);
+
+			if (g_slist_length(msg_list) == 0) {
+				g_free(entries[len]);
+				continue;
+			}
+
+			sscanf(dir->d_name, "%*u-%lu-%as", &flags, &uuid);
+
+			oldpath = g_strdup_printf("%s/%s", path, dir->d_name);
+
+			g_free(entries[len]);
+
+			entry = g_try_new0(struct txq_backup_entry, 1);
+			if (entry == NULL) {
+				g_free(oldpath);
+				continue;
+			}
+
+			entry->msg_list = msg_list;
+			entry->uuid = uuid;
+			entry->flags = flags;
+			entry->oldpath = oldpath;
+
+			g_queue_push_head(retq, entry);
+		}
+
+		g_free(path);
+		g_free(entries);
+	}
+
+	/* rename directory to reflect new position in queue */
+	for (i = 0; i < g_queue_get_length(retq); i++) {
+		char *newpath;
+		struct txq_backup_entry *entry = g_queue_peek_nth(retq, i);
+
+		newpath = g_strdup_printf(SMS_TX_BACKUP_PATH_DIR,
+                                                        imsi, i, entry->flags,
+							entry->uuid);
+
+		rename(entry->oldpath, newpath);
+
+		g_free(newpath);
+		g_free(entry->oldpath);
+	}
+
+	return retq;
+}
+
 gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
 			const char *uuid, struct sms *s, guint8 seq)
 {
diff --git a/src/smsutil.h b/src/smsutil.h
index fa4e453..38d8dc9 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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,
-- 
1.7.2.3

[PATCH 0/3] Persist TX SMS messages

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 806 bytes --]

This patch series is a re-sending of a previous series sent by Kristen Carlson
Accardi. What I did was some cleanups, style changes, ported it to the new sms
API and fixed some leaks (hopefully I didn't introduce more).

Marcel/Denis, I don't know if letting "Based on patch from" is the right
approach. Feel free to change the author of the commits if you think this more
appropriate.

Lucas De Marchi (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     |  107 +++++++++++++++++++++++++++
 src/smsutil.c |  223 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |   16 ++++
 4 files changed, 347 insertions(+), 0 deletions(-)

-- 
1.7.3.5

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

[Lucas De Marchi]

[-- Attachment #1: Type: text/plain, Size: 8170 bytes --]

Based on patch from: Kristen Carlson Accardi <kristen@linux.intel.com>
---
 src/ofono.h   |    1 +
 src/sms.c     |   83 +++++++++++++++++++++++++++++++
 src/smsutil.c |  151 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/smsutil.h |    8 +++
 4 files changed, 243 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index e48dbf6..14f958e 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -245,6 +245,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);
diff --git a/src/sms.c b/src/sms.c
index 403e387..1c3109c 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -798,6 +798,9 @@ static struct tx_queue_entry *tx_queue_entry_new(GSList *msg_list,
 				pdu->pdu_len, pdu->tpdu_len);
 	}
 
+	if (flags & OFONO_SMS_SUBMIT_FLAG_REUSE_UUID)
+		return entry;
+
 	if (sms_uuid_from_pdus(entry->pdus, entry->num_pdus, &entry->uuid))
 		return entry;
 
@@ -1694,6 +1697,84 @@ 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;
+	struct txq_backup_entry *backup_entry;
+	struct ofono_uuid ofono_uuid;
+
+	DBG("");
+
+	backupq = sms_tx_queue_load(sms->imsi);
+
+	if (backupq == NULL)
+		return;
+
+	while ((backup_entry = g_queue_pop_head(backupq))) {
+		struct message *m;
+		struct tx_queue_entry *txq_entry;
+
+		decode_hex_own_buf(backup_entry->uuid, -1, NULL, 0,
+							ofono_uuid.uuid);
+
+		DBG("Trying to restore: %s", ofono_uuid.uuid);
+
+		backup_entry->flags |= OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+		txq_entry = tx_queue_entry_new(backup_entry->msg_list,
+							backup_entry->flags);
+		if (txq_entry == NULL)
+			goto loop_out;
+
+		txq_entry->flags &= ~OFONO_SMS_SUBMIT_FLAG_REUSE_UUID;
+		memcpy(&txq_entry->uuid, &ofono_uuid, sizeof(ofono_uuid));
+
+		m = message_create(&txq_entry->uuid, sms->atom);
+		if (m == NULL) {
+			tx_queue_entry_destroy(txq_entry);
+
+			goto loop_out;
+		}
+
+		if (message_dbus_register(m) == FALSE) {
+			tx_queue_entry_destroy(txq_entry);
+
+			goto loop_out;
+		}
+
+		message_set_data(m, txq_entry);
+		g_hash_table_insert(sms->messages, &txq_entry->uuid, m);
+
+		if (backup_entry->msg_list->next != NULL) {
+			if (sms->ref == 65536)
+				sms->ref = 1;
+			else
+				sms->ref = sms->ref + 1;
+		}
+
+		txq_entry->id = sms->tx_counter++;
+		g_queue_push_tail(sms->txq, txq_entry);
+
+		/*
+		 * only messsages with OFONO_SMS_SUBMIT_FLAG_EXPOSE_DBUS flag
+		 * are restored, so we can safely emit here without checking
+		 * by this flag
+		 */
+		message_emit_added(m, OFONO_MESSAGE_MANAGER_INTERFACE);
+
+loop_out:
+		g_slist_foreach(backup_entry->msg_list, (GFunc)g_free, NULL);
+		g_slist_free(backup_entry->msg_list);
+
+		g_free(backup_entry->uuid);
+		g_free(backup_entry);
+	}
+
+	if (g_queue_get_length(sms->txq) > 0)
+		sms->tx_source = g_timeout_add(0, tx_next, sms);
+
+	g_queue_free(backupq);
+}
+
 /*
  * Indicate oFono that a SMS driver is ready for operation
  *
@@ -1766,6 +1847,8 @@ void ofono_sms_register(struct ofono_sms *sms)
 		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);
 
diff --git a/src/smsutil.c b/src/smsutil.c
index f4e3049..04759f0 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -2327,6 +2327,15 @@ static gboolean sms_deserialize(const unsigned char *buf,
 	return sms_decode(buf + 1, len - 1, FALSE, buf[0], sms);
 }
 
+static gboolean sms_deserialize_outgoing(const unsigned char *buf,
+		struct sms *sms, int len)
+{
+	if (len < 1)
+		return FALSE;
+
+	return sms_decode(buf + 1, len - 1, TRUE, buf[0], sms);
+}
+
 static gboolean sms_assembly_extract_address(const char *straddr,
 						struct sms_address *out)
 {
@@ -3147,6 +3156,148 @@ void status_report_assembly_expire(struct status_report_assembly *assembly,
 	}
 }
 
+/*
+ * Each directory contains a file per pdu.
+ */
+static GSList *sms_tx_load(const char *imsi, const struct dirent *dir)
+{
+	GSList *list = NULL;
+	struct dirent **pdus;
+	char *path;
+	int len, r;
+	unsigned char buf[177];
+	struct sms s;
+
+	if (dir->d_type != DT_DIR)
+		return NULL;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH "/%s", imsi, dir->d_name);
+	len = scandir(path, &pdus, NULL, versionsort);
+	g_free(path);
+
+	if (len < 0)
+		return NULL;
+
+	while (len-- > 2) {
+		if (pdus[len]->d_type != DT_REG)
+			goto free_pdu;
+
+		r = read_file(buf, sizeof(buf), SMS_TX_BACKUP_PATH "/%s/%s",
+					imsi, dir->d_name, pdus[len]->d_name);
+
+		if (r < 0)
+			goto free_pdu;
+
+		if (sms_deserialize_outgoing(buf, &s, r) == FALSE)
+			goto free_pdu;
+
+		list = g_slist_prepend(list, g_memdup(&s, sizeof(s)));
+
+free_pdu:
+		g_free(pdus[len]);
+	}
+
+	g_free(pdus[1]);
+	g_free(pdus[0]);
+	g_free(pdus);
+
+	return list;
+}
+
+/*
+ * populate the queue with tx_backup_entry from stored backup
+ * data.
+ */
+GQueue *sms_tx_queue_load(const char *imsi)
+{
+	char *path;
+	GQueue *retq;
+	struct dirent **entries;
+	int len;
+	GSList *msg_list;
+	unsigned long flags;
+	unsigned long i;
+
+	if (imsi == NULL)
+		return NULL;
+
+	path = g_strdup_printf(SMS_TX_BACKUP_PATH, imsi);
+	if (path == NULL)
+		goto nomem_path;
+
+	retq = g_queue_new();
+	if (retq == NULL)
+		goto nomem_retq;
+
+	len = scandir(path, &entries, NULL, versionsort);
+
+	if (len < 0)
+		goto nodir_exit;
+
+	while (len-- > 2) {
+		char *uuid;
+		char *oldpath;
+		struct txq_backup_entry *entry;
+		struct dirent *dir = entries[len];
+
+		entry = g_try_new0(struct txq_backup_entry, 1);
+		if (entry == NULL)
+			goto free_dir;
+
+		msg_list = sms_tx_load(imsi, dir);
+
+		if (msg_list == NULL) {
+			g_free(entry);
+
+			goto free_dir;
+		}
+
+		sscanf(dir->d_name, "%*u-%lu-%as", &flags, &uuid);
+		oldpath = g_strdup_printf("%s/%s", path, dir->d_name);
+
+		entry->msg_list = msg_list;
+		entry->uuid = uuid;
+		entry->flags = flags;
+		entry->oldpath = oldpath;
+
+		g_queue_push_head(retq, entry);
+
+free_dir:
+		g_free(dir);
+	}
+
+	g_free(entries[1]);
+	g_free(entries[0]);
+	g_free(entries);
+	g_free(path);
+
+	/* rename directory to reflect new position in queue */
+	for (i = 0; i < g_queue_get_length(retq); i++) {
+		char *newpath;
+		struct txq_backup_entry *entry = g_queue_peek_nth(retq, i);
+
+		newpath = g_strdup_printf(SMS_TX_BACKUP_PATH_DIR,
+                                                        imsi, i, entry->flags,
+							entry->uuid);
+
+		rename(entry->oldpath, newpath);
+
+		g_free(newpath);
+		g_free(entry->oldpath);
+	}
+
+	return retq;
+
+nodir_exit:
+	g_queue_free(retq);
+
+nomem_retq:
+	g_free(path);
+
+nomem_path:
+	return NULL;
+}
+
 gboolean sms_tx_store(const char *imsi, unsigned long id, unsigned long flags,
 			const char *uuid, struct sms *s, guint8 seq)
 {
diff --git a/src/smsutil.h b/src/smsutil.h
index 3d3b72a..9e56c5f 100644
--- a/src/smsutil.h
+++ b/src/smsutil.h
@@ -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,
-- 
1.7.3.5