Hi Kristen, On 11/24/2010 04:53 PM, Kristen Carlson Accardi wrote: 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. Is there a reason for the type of id and the type of tx_counter to be different? I'd really prefer i = 0, l = list here And where are you taking care of storing the other needed information? e.g. the tx_queue entry flags, retry count, etc? Otherwise I'm happy with this one. Regards, -Denis

Hi Kristen, 2010/11/25 Kristen Carlson Accardi <kristen(a)linux.intel.com&gt;: 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). Need to explicitly compare to NULL here. A recently added convention, I admit. Cheers, Aki

Hi Kristen, On 11/24/2010 04:53 PM, Kristen Carlson Accardi wrote: 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. The existing code tends to name this foo_backup_free rather than backup_destroy. Rule M1 please Rule M1 please Regards, -Denis

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

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