* RE: [RFC PATCH 5/5] smsutil: save pending status reports to imsi file
[not found] <FEE7A63B35E92D4F93D1BB82DE254AE5021987@JKLMAIL02.ixonos.local>
@ 2010-08-10 16:25 ` Tikander Petteri
2010-08-12 17:24 ` Denis Kenzior
0 siblings, 1 reply; 5+ messages in thread
From: Tikander Petteri @ 2010-08-10 16:25 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 15836 bytes --]
Hi Denis,
I'm the guy, who already introduced shortly yesterday to Inaky, and I'm continuing Pasi Miettinen's fine
work with SMS status-reports.
So, here are some comments (included below) for updating status report-functionality for your earlier comments,
how SR-functionality could be improved.
Br, Petteri
> ________________________________________
> Lähettäjä: ofono-bounces(a)ofono.org [ofono-bounces(a)ofono.org]
> käyttäjän ext Denis Kenzior [denkenz(a)gmail.com] puolesta
> Lähetetty: 22. kesäkuuta 2010 0:07
> Vastaanottaja: ofono(a)ofono.org
> Aihe: Re: [RFC PATCH 5/5] smsutil: save pending status reports to
> imsi file
>
> Hi Pasi,
>
> > ---
> > src/smsutil.c | 205
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files
> changed,
> > 204 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/smsutil.c b/src/smsutil.c
> > index 3db3d28..a30a281 100644
> > --- a/src/smsutil.c
> > +++ b/src/smsutil.c
> > @@ -45,6 +45,10 @@
> > #define SMS_BACKUP_PATH_DIR SMS_BACKUP_PATH "/%s-%i-%i"
> > #define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR "/%03i"
> >
> > +#define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
> > +#define SMS_SR_BACKUP_PATH_DIR SMS_SR_BACKUP_PATH "/%s-%i-%i"
> > +#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH_DIR "/%i"
> > +
> > #define SMS_ADDR_FMT "%24[0-9A-F]"
> >
> > static GSList *sms_assembly_add_fragment_backup(struct sms_assembly
> > *assembly, @@ -2625,20 +2629,209 @@ void sms_assembly_expire(struct
> > sms_assembly *assembly, time_t before) }
> > }
> >
> > +static void sr_assembly_load_backup(GHashTable *assembly_table,
> > + const char *imsi,
> > + const struct dirent *addr_dir)
> > +{
> > + char *path;
> > + struct dirent **ids;
> > + struct sms_address *addr;
> > + struct id_table_node *node;
> > + GHashTable *id_table;
> > + int len;
> > + int r;
> > + unsigned char buf[sizeof(node->mrs) + sizeof(node->total_mrs) +
> > + sizeof(node->sent_mrs) + sizeof(node->deliverable)];
> > + char *assembly_table_key;
> > + unsigned int *id_table_key;
> > + struct stat segment_stat;
> > +
> > + if (addr_dir->d_type != DT_DIR)
> > + return;
> > +
> > + addr = g_new0(struct sms_address, 1);
>
> Why are you mallocing addr here? Seems that this only makes the code
> more
> complicated.
OK, I will change that, so that mallocs (and freeings) not needed anymore for addr.
>
> > +
> > + if (sscanf(addr_dir->d_name, "%[0-9]-%i-%i",
> > + addr->address, (int *) &addr->number_type,
> > + (int *) &addr->numbering_plan) < 3) {
>
> Lets make this consistent with sms assembly, please use SMS_ADDR_FMT.
> You're
> also free to use sms_assembly utility functions if that helps you.
>
OK.
> > + g_free(addr);
> > + return;
> > + }
> > +
> > + /* Go through different msg_ids. */
> > + path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", imsi,
> > + addr_dir->d_name);
> > + len = scandir(path, &ids, NULL, versionsort);
> > +
> > + g_free(path);
> > +
> > + if (len < 0) {
> > + g_free(addr);
> > + return;
> > + }
> > +
> > + id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> > + g_free, g_free);
> > +
> > + assembly_table_key = g_try_malloc(sizeof(addr->address));
> > +
> > + if (assembly_table_key == NULL) {
> > + g_free(addr);
> > + return;
> > + }
> > +
> > + g_strlcpy(assembly_table_key, addr->address, sizeof(addr->address));
> > + g_hash_table_insert(assembly_table, assembly_table_key, id_table);
> > +
> > + while (len--) {
> > + path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s/%s",
> > + imsi, addr_dir->d_name, ids[len]->d_name);
> > + r = read_file(buf, sizeof(buf), SMS_SR_BACKUP_PATH "/%s/%s",
> > + imsi, addr_dir->d_name, ids[len]->d_name);
> > +
> > + if (r < 0) {
> > + g_free(path);
> > + g_free(ids[len]);
> > + continue;
> > + }
> > +
> > + r = stat(path, &segment_stat);
> > +
> > + if (r != 0) {
> > + g_free(path);
> > + g_free(ids[len]);
> > + continue;
> > + }
> > + /* Gather the data for id_table node */
> > + node = g_new0(struct id_table_node, 1);
> > + node->to = *addr;
>
> Please use memcpy for copying structures.
>
OK.
> > + node->expiration = segment_stat.st_mtime;
> > + memcpy(node->mrs, buf, sizeof(node->mrs));
> > + memcpy(&node->total_mrs, buf + sizeof(node->mrs),
> > + sizeof(node->total_mrs));
> > + memcpy(&node->sent_mrs,
> > + buf + sizeof(node->mrs) + sizeof(node->total_mrs),
> > + sizeof(node->sent_mrs));
> > +
> > + memcpy(&node->deliverable, buf + sizeof(node->mrs) +
> > + sizeof(node->total_mrs) + sizeof(node->sent_mrs),
> > + sizeof(node->deliverable));
> > + /* Node ready, create key and add them to the table */
> > + id_table_key = g_new0(unsigned int, 1);
> > + *id_table_key = atoi(ids[len]->d_name);
> > +
> > + g_hash_table_insert(id_table, id_table_key, node);
> > +
> > + g_free(path);
> > + g_free(ids[len]);
> > + }
> > + g_free(addr);
> > + g_free(ids);
> > +}
> > +
>
> General comment here is that you're duplicating much of the code that
> inserts
> the information into the data structure. You can play the same trick as
> sms_assembly which uses a single function to do the heavy lifting,
> but takes
> an argument whether to store this info on disk. Obviously during
> load you
> don't want to re-save this information on disk. Then your load function
> becomes much smaller and easier to understand.
>
OK. I agree that generally single function for heavy lifting is nicer solution.
I will implement some general function, which handles adding of new id-table (if not already added)
to assembly-table, and storing necessary node-information there. After that I will move some functionality from
sr_assembly_load_backup() to this general function. Also this function can be used when sending new sms-text message.
And I will add new parameter for telling, if this function will save data to imsi-file on disk or not.
Existing sms-assembly handles it that way. But, one difference with sms-assembly and status-report logic is, that
sms-assembly naturally stores every messages of concatenated, long SMS-message to own backup files, but status report
gathers all information belonging to the same long SMS-message (using message reference numbers) to the single file.
So Pasi implemented in the loading-phase (sr_assembly_load_backup) storing of MR-numbers simply
by copying the whole buffer containing MR's belonging to the single concatenated message-node:
1) memcpy(node->mrs, buf, sizeof(node->mrs));
And naturally when sending single message, single MR-bit is set up ('status_report_assembly_add_fragment()'):
2) node->mrs[offset] |= bit;
So, when now implementing new general function, one solution could be to bring MR-string as parameter
so way (1) could be used in different cases.
Also I have to take care of variable 'sent_mrs', which will be incremented in the sms-submit, but not in the loading phase.
> > struct status_report_assembly *status_report_assembly_new(const char
> > *imsi) {
> > + char *path;
> > + int len;
> > + struct dirent **addresses;
> > struct status_report_assembly *ret =
> > g_new0(struct status_report_assembly, 1);
> >
> > ret->assembly_table = g_hash_table_new_full(g_str_hash, g_str_equal,
> > g_free,
> (GDestroyNotify)g_hash_table_destroy);
> >
> > - if (imsi)
> > + if (imsi) {
> > ret->imsi = imsi;
> >
> > + /* Restore state from backup */
> > + path = g_strdup_printf(SMS_SR_BACKUP_PATH, imsi);
> > + len = scandir(path, &addresses, NULL, alphasort);
> > +
> > + g_free(path);
> > +
> > + if (len < 0)
> > + return ret;
>
> There should be an empty line here
>
OK.
> > + /* Go through different addresses. Each address can relate
> to
> > + * 1-n msg_ids. Do not try to load . and .. directories.
> > + */
> > + g_free(addresses[0]);
> > + g_free(addresses[1]);
>
> This really isn't necessary, the '.' and '..' directories will not
> pass the
> sscanf test above.
>
OK.
> > +
> > + while (2 < len--) {
> > + sr_assembly_load_backup(ret->assembly_table, imsi,
> > +
> addresses[len]);
> > + g_free(addresses[len]);
> > + }
>
> Empty line here please
OK.
>
> > + g_free(addresses);
> > + }
>
> And another empty line here. Please put an empty line after each block
> unless
> it is at the end of the function.
OK.
> > return ret;
> > }
> >
> > +static gboolean sr_assembly_add_fragment_backup(const char *imsi,
> > + const struct id_table_node *node,
> > + unsigned int msg_id)
>
> Lets be consistent with how sms assembly works. E.g. take the main
> assembly
> object and return void. Just helps to read the code when it is
> consistent.
>
OK. Lets give main assembly to this (and corresponding) function(s).
How about return value? For instance existing sms assembly-function 'sms_assembly_add_fragment_backup()'
already returns something else than void.
> > +{
> > + int len = sizeof(node->mrs) + sizeof(node->total_mrs) +
> > + sizeof(node->sent_mrs) + sizeof(node->deliverable);
> > + unsigned char buf[len];
> > +
> > + if (!imsi)
> > + return FALSE;
> > +
> > + memcpy(buf, node->mrs, sizeof(node->mrs));
> > +
> > + memcpy(buf + sizeof(node->mrs), &node->total_mrs,
> > + sizeof(node->total_mrs));
> > +
> > + memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs),
> > + &node->sent_mrs, sizeof(node->sent_mrs));
> > +
> > + memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs) +
> > + sizeof(node->sent_mrs), &node->deliverable,
> sizeof(node->deliverable));
> > +
> > + /* storagedir/%s/sms_sr/%s-%i-%i/%i */
> > + if (write_file(buf, len, SMS_BACKUP_MODE, SMS_SR_BACKUP_PATH_FILE,
> imsi,
> > + node->to.address, node->to.number_type,
> > + node->to.numbering_plan, msg_id) != len)
> > + return FALSE;
> > +
> > + return TRUE;
> > +}
> > +
> > +static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
> > + const struct id_table_node *node,
> > + unsigned int msg_id)
>
> Same consistency comment here. Take main assembly object and return
> void.
>
> > +{
> > + char *path;
> > +
> > + if (!imsi)
> > + return FALSE;
> > +
> > + path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, imsi,
> node->to.address,
> > + node->to.number_type,
> node->to.numbering_plan,
> > + msg_id);
> > +
> > + unlink(path);
> > + g_free(path);
> > +
> > + path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, imsi,
> node->to.address,
> > + node->to.number_type,
> node->to.numbering_plan);
> > +
> > + /* If the address does not have relating msg_ids anymore, remove it
> */
> > + rmdir(path);
> > + g_free(path);
> > +
> > + return TRUE;
> > +}
> > +
> > +static gboolean sr_assembly_update_fragment_backup(const char *imsi,
> > + const struct id_table_node *node,
> > + unsigned int msg_id)
> > +{
> > + return sr_assembly_remove_fragment_backup(imsi, node, msg_id) &&
> > + sr_assembly_add_fragment_backup(imsi, node, msg_id);
> > +}
> > +
>
> Is this function really necessary? write_file overwrites the file,
> so simply
> using add_fragment_backup seems sufficient.
>
OK. Removal of backup file is not necessary, so I will handle updating of existing file simple
by overwriting the existing file.
> > void status_report_assembly_free(struct status_report_assembly *assembly)
> > {
> > g_hash_table_destroy(assembly->assembly_table);
> > @@ -2707,10 +2900,14 @@ gboolean status_report_assembly_report(struct
> > status_report_assembly *assembly, * not delete the node yet.
> > */
> > if (pending) {
> > + sr_assembly_update_fragment_backup(assembly->imsi,
> node,
> > + *msg_id);
>
> It seems to me fragment is the wrong name here. What about
> 'store_node_backup'?
>
> > *msg_delivered = FALSE;
> > return update_history;
> > } else {
> > *msg_delivered = node->deliverable;
> > + sr_assembly_remove_fragment_backup(assembly->imsi,
> node,
> > + *msg_id);
>
> maybe 'remove_node_backup'
>
> >
> > g_hash_table_iter_remove(&iter);
> >
> > @@ -2747,6 +2944,7 @@ void status_report_assembly_add_fragment(
> > unsigned int *id_table_key;
> >
> > id_table = g_hash_table_lookup(assembly->assembly_table,
> to->address);
> > +
> > /* Create id_table and node */
> > if (id_table == NULL) {
> > id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> > @@ -2775,6 +2973,9 @@ void status_report_assembly_add_fragment(
> >
> > g_hash_table_insert(assembly->assembly_table,
> > assembly_table_key, id_table);
> > +
> > + sr_assembly_add_fragment_backup(assembly->imsi, node,
> msg_id);
> > +
> > return;
> > }
> >
> > @@ -2793,12 +2994,14 @@ void status_report_assembly_add_fragment(
> > *id_table_key = msg_id;
> > g_hash_table_insert(id_table, id_table_key, node);
> >
> > + sr_assembly_add_fragment_backup(assembly->imsi, node,
> msg_id);
> > return;
> > }
> > /* id_table and node both exists */
> > node->mrs[offset] |= bit;
> > node->expiration = expiration;
> > node->sent_mrs++;
> > + sr_assembly_update_fragment_backup(assembly->imsi, node, msg_id);
> > }
> >
> > void status_report_assembly_expire(struct status_report_assembly
> > *assembly,
> >
>
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RFC PATCH 5/5] smsutil: save pending status reports to imsi file
2010-08-10 16:25 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Tikander Petteri
@ 2010-08-12 17:24 ` Denis Kenzior
0 siblings, 0 replies; 5+ messages in thread
From: Denis Kenzior @ 2010-08-12 17:24 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3649 bytes --]
Hi Petteri,
On 08/10/2010 11:25 AM, Tikander Petteri wrote:
> Hi Denis,
>
> I'm the guy, who already introduced shortly yesterday to Inaky, and I'm continuing Pasi Miettinen's fine
> work with SMS status-reports.
Good to have you aboard :)
> So, here are some comments (included below) for updating status report-functionality for your earlier comments,
> how SR-functionality could be improved.
>
<snip>
>> General comment here is that you're duplicating much of the code that
>> inserts
>> the information into the data structure. You can play the same trick as
>> sms_assembly which uses a single function to do the heavy lifting,
>> but takes
>> an argument whether to store this info on disk. Obviously during
>> load you
>> don't want to re-save this information on disk. Then your load function
>> becomes much smaller and easier to understand.
>>
>
> OK. I agree that generally single function for heavy lifting is nicer solution.
> I will implement some general function, which handles adding of new id-table (if not already added)
> to assembly-table, and storing necessary node-information there. After that I will move some functionality from
> sr_assembly_load_backup() to this general function. Also this function can be used when sending new sms-text message.
> And I will add new parameter for telling, if this function will save data to imsi-file on disk or not.
> Existing sms-assembly handles it that way. But, one difference with sms-assembly and status-report logic is, that
> sms-assembly naturally stores every messages of concatenated, long SMS-message to own backup files, but status report
> gathers all information belonging to the same long SMS-message (using message reference numbers) to the single file.
>
> So Pasi implemented in the loading-phase (sr_assembly_load_backup) storing of MR-numbers simply
> by copying the whole buffer containing MR's belonging to the single concatenated message-node:
>
> 1) memcpy(node->mrs, buf, sizeof(node->mrs));
>
> And naturally when sending single message, single MR-bit is set up ('status_report_assembly_add_fragment()'):
>
> 2) node->mrs[offset] |= bit;
>
> So, when now implementing new general function, one solution could be to bring MR-string as parameter
> so way (1) could be used in different cases.
>
> Also I have to take care of variable 'sent_mrs', which will be incremented in the sms-submit, but not in the loading phase.
>
I agree there are going to be differences. See how far you get trying
to unify these behind a single function and send for another round of
reviews.
<snip>
>>> return ret;
>>> }
>>>
>>> +static gboolean sr_assembly_add_fragment_backup(const char *imsi,
>>> + const struct id_table_node *node,
>>> + unsigned int msg_id)
>>
>> Lets be consistent with how sms assembly works. E.g. take the main
>> assembly
>> object and return void. Just helps to read the code when it is
>> consistent.
>>
>
> OK. Lets give main assembly to this (and corresponding) function(s).
> How about return value? For instance existing sms assembly-function 'sms_assembly_add_fragment_backup()'
> already returns something else than void.
>
sms_assembly_add_fragment_backup returns the new list of fragments.
What value can we return from this function? I think returning
TRUE/FALSE for file write failures are not worth it. There's not much
that can be done in that case anyway... So it seems void is good
enough, but feel free to suggest something.
Regards,
-Denis
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 4/5] sms: Status report notify
@ 2010-06-17 13:14 Pasi Miettinen
2010-06-17 13:14 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Pasi Miettinen
0 siblings, 1 reply; 5+ messages in thread
From: Pasi Miettinen @ 2010-06-17 13:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]
---
src/sms.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 70 insertions(+), 1 deletions(-)
diff --git a/src/sms.c b/src/sms.c
index bf6d261..e6a2dca 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -68,6 +68,8 @@ struct ofono_sms {
void *driver_data;
struct ofono_atom *atom;
ofono_bool_t use_delivery_reports;
+ struct status_report_assembly *sr_assembly;
+
};
struct pending_pdu {
@@ -83,6 +85,8 @@ struct tx_queue_entry {
unsigned int msg_id;
unsigned int retry;
DBusMessage *msg;
+ gboolean status_report;
+ struct sms_address receiver;
};
static void set_sca(struct ofono_sms *sms,
@@ -331,6 +335,13 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
entry->cur_pdu += 1;
entry->retry = 0;
+ if (entry->status_report)
+ status_report_assembly_add_fragment(sms->sr_assembly,
+ entry->msg_id,
+ &entry->receiver,
+ mr, time(NULL),
+ entry->num_pdus);
+
if (entry->cur_pdu < entry->num_pdus) {
sms->tx_source = g_timeout_add(0, tx_next, sms);
return;
@@ -462,6 +473,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
set_ref_and_to(msg_list, sms->ref, ref_offset, to);
entry = create_tx_queue_entry(msg_list);
+ sms_address_from_string(&entry->receiver, to);
+
g_slist_foreach(msg_list, (GFunc)g_free, NULL);
g_slist_free(msg_list);
@@ -474,6 +487,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
entry->msg = dbus_message_ref(msg);
entry->msg_id = sms->next_msg_id++;
+ entry->status_report = sms->use_delivery_reports;
g_queue_push_tail(sms->txq, entry);
@@ -718,6 +732,30 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms *incoming)
g_slist_free(l);
}
+static void handle_sms_status_report(struct ofono_sms *sms,
+ const struct sms *incoming)
+{
+ gboolean delivered;
+ unsigned int msg_id;
+ gboolean update_history;
+ struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
+
+ update_history = status_report_assembly_report(sms->sr_assembly,
+ incoming, &msg_id, &delivered);
+
+ if (update_history) {
+
+ if (delivered)
+ __ofono_history_sms_send_status(modem, msg_id,
+ time(NULL), OFONO_HISTORY_SMS_STATUS_DELIVERED);
+ else
+ __ofono_history_sms_send_status(modem, msg_id,
+ time(NULL),
+ OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED);
+ }
+}
+
+
static inline gboolean handle_mwi(struct ofono_sms *sms, struct sms *s)
{
gboolean discard;
@@ -849,7 +887,30 @@ out:
void ofono_sms_status_notify(struct ofono_sms *sms, unsigned char *pdu,
int len, int tpdu_len)
{
- ofono_error("SMS Status-Report not yet handled");
+ struct sms s;
+ enum sms_class cls;
+
+ if (!sms_decode(pdu, len, FALSE, tpdu_len, &s)) {
+ ofono_error("Unable to decode PDU");
+ return;
+ }
+
+ if (s.type != SMS_TYPE_STATUS_REPORT) {
+ ofono_error("Expecting a STATUS REPORT pdu");
+ return;
+ }
+
+ if (s.status_report.srq) {
+ ofono_error("Waiting an answer to SMS-SUBMIT, not SMS-COMMAND");
+ return;
+ }
+
+ if (!sms_dcs_decode(s.deliver.dcs, &cls, NULL, NULL, NULL)) {
+ ofono_error("Unknown / Reserved DCS. Ignoring");
+ return;
+ }
+
+ handle_sms_status_report(sms, &s);
}
int ofono_sms_driver_register(const struct ofono_sms_driver *d)
@@ -932,6 +993,11 @@ static void sms_remove(struct ofono_atom *atom)
sms->settings = NULL;
}
+ if (sms->sr_assembly) {
+ status_report_assembly_free(sms->sr_assembly);
+ sms->sr_assembly = NULL;
+ }
+
g_free(sms);
}
@@ -1069,9 +1135,12 @@ void ofono_sms_register(struct ofono_sms *sms)
imsi = ofono_sim_get_imsi(sms->sim);
sms->assembly = sms_assembly_new(imsi);
+ sms->sr_assembly = status_report_assembly_new(imsi);
+
sms_load_settings(sms, imsi);
} else {
sms->assembly = sms_assembly_new(NULL);
+ sms->sr_assembly = status_report_assembly_new(NULL);
}
__ofono_atom_register(sms->atom, sms_unregister);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC PATCH 5/5] smsutil: save pending status reports to imsi file
2010-06-17 13:14 [RFC PATCH 4/5] sms: Status report notify Pasi Miettinen
@ 2010-06-17 13:14 ` Pasi Miettinen
2010-06-21 21:07 ` Denis Kenzior
0 siblings, 1 reply; 5+ messages in thread
From: Pasi Miettinen @ 2010-06-17 13:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8069 bytes --]
---
src/smsutil.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 204 insertions(+), 1 deletions(-)
diff --git a/src/smsutil.c b/src/smsutil.c
index 3db3d28..a30a281 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -45,6 +45,10 @@
#define SMS_BACKUP_PATH_DIR SMS_BACKUP_PATH "/%s-%i-%i"
#define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR "/%03i"
+#define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
+#define SMS_SR_BACKUP_PATH_DIR SMS_SR_BACKUP_PATH "/%s-%i-%i"
+#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH_DIR "/%i"
+
#define SMS_ADDR_FMT "%24[0-9A-F]"
static GSList *sms_assembly_add_fragment_backup(struct sms_assembly *assembly,
@@ -2625,20 +2629,209 @@ void sms_assembly_expire(struct sms_assembly *assembly, time_t before)
}
}
+static void sr_assembly_load_backup(GHashTable *assembly_table,
+ const char *imsi,
+ const struct dirent *addr_dir)
+{
+ char *path;
+ struct dirent **ids;
+ struct sms_address *addr;
+ struct id_table_node *node;
+ GHashTable *id_table;
+ int len;
+ int r;
+ unsigned char buf[sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs) + sizeof(node->deliverable)];
+ char *assembly_table_key;
+ unsigned int *id_table_key;
+ struct stat segment_stat;
+
+ if (addr_dir->d_type != DT_DIR)
+ return;
+
+ addr = g_new0(struct sms_address, 1);
+
+ if (sscanf(addr_dir->d_name, "%[0-9]-%i-%i",
+ addr->address, (int *) &addr->number_type,
+ (int *) &addr->numbering_plan) < 3) {
+ g_free(addr);
+ return;
+ }
+
+ /* Go through different msg_ids. */
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", imsi,
+ addr_dir->d_name);
+ len = scandir(path, &ids, NULL, versionsort);
+
+ g_free(path);
+
+ if (len < 0) {
+ g_free(addr);
+ return;
+ }
+
+ id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
+ g_free, g_free);
+
+ assembly_table_key = g_try_malloc(sizeof(addr->address));
+
+ if (assembly_table_key == NULL) {
+ g_free(addr);
+ return;
+ }
+
+ g_strlcpy(assembly_table_key, addr->address, sizeof(addr->address));
+ g_hash_table_insert(assembly_table, assembly_table_key, id_table);
+
+ while (len--) {
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s/%s",
+ imsi, addr_dir->d_name, ids[len]->d_name);
+ r = read_file(buf, sizeof(buf), SMS_SR_BACKUP_PATH "/%s/%s",
+ imsi, addr_dir->d_name, ids[len]->d_name);
+
+ if (r < 0) {
+ g_free(path);
+ g_free(ids[len]);
+ continue;
+ }
+
+ r = stat(path, &segment_stat);
+
+ if (r != 0) {
+ g_free(path);
+ g_free(ids[len]);
+ continue;
+ }
+ /* Gather the data for id_table node */
+ node = g_new0(struct id_table_node, 1);
+ node->to = *addr;
+ node->expiration = segment_stat.st_mtime;
+ memcpy(node->mrs, buf, sizeof(node->mrs));
+ memcpy(&node->total_mrs, buf + sizeof(node->mrs),
+ sizeof(node->total_mrs));
+ memcpy(&node->sent_mrs,
+ buf + sizeof(node->mrs) + sizeof(node->total_mrs),
+ sizeof(node->sent_mrs));
+
+ memcpy(&node->deliverable, buf + sizeof(node->mrs) +
+ sizeof(node->total_mrs) + sizeof(node->sent_mrs),
+ sizeof(node->deliverable));
+ /* Node ready, create key and add them to the table */
+ id_table_key = g_new0(unsigned int, 1);
+ *id_table_key = atoi(ids[len]->d_name);
+
+ g_hash_table_insert(id_table, id_table_key, node);
+
+ g_free(path);
+ g_free(ids[len]);
+ }
+ g_free(addr);
+ g_free(ids);
+}
+
struct status_report_assembly *status_report_assembly_new(const char *imsi)
{
+ char *path;
+ int len;
+ struct dirent **addresses;
struct status_report_assembly *ret =
g_new0(struct status_report_assembly, 1);
ret->assembly_table = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, (GDestroyNotify)g_hash_table_destroy);
- if (imsi)
+ if (imsi) {
ret->imsi = imsi;
+ /* Restore state from backup */
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH, imsi);
+ len = scandir(path, &addresses, NULL, alphasort);
+
+ g_free(path);
+
+ if (len < 0)
+ return ret;
+ /* Go through different addresses. Each address can relate to
+ * 1-n msg_ids. Do not try to load . and .. directories.
+ */
+ g_free(addresses[0]);
+ g_free(addresses[1]);
+
+ while (2 < len--) {
+ sr_assembly_load_backup(ret->assembly_table, imsi,
+ addresses[len]);
+ g_free(addresses[len]);
+ }
+ g_free(addresses);
+ }
return ret;
}
+static gboolean sr_assembly_add_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ int len = sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs) + sizeof(node->deliverable);
+ unsigned char buf[len];
+
+ if (!imsi)
+ return FALSE;
+
+ memcpy(buf, node->mrs, sizeof(node->mrs));
+
+ memcpy(buf + sizeof(node->mrs), &node->total_mrs,
+ sizeof(node->total_mrs));
+
+ memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs),
+ &node->sent_mrs, sizeof(node->sent_mrs));
+
+ memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs), &node->deliverable, sizeof(node->deliverable));
+
+ /* storagedir/%s/sms_sr/%s-%i-%i/%i */
+ if (write_file(buf, len, SMS_BACKUP_MODE, SMS_SR_BACKUP_PATH_FILE, imsi,
+ node->to.address, node->to.number_type,
+ node->to.numbering_plan, msg_id) != len)
+ return FALSE;
+
+ return TRUE;
+}
+
+static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ char *path;
+
+ if (!imsi)
+ return FALSE;
+
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, imsi, node->to.address,
+ node->to.number_type, node->to.numbering_plan,
+ msg_id);
+
+ unlink(path);
+ g_free(path);
+
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, imsi, node->to.address,
+ node->to.number_type, node->to.numbering_plan);
+
+ /* If the address does not have relating msg_ids anymore, remove it */
+ rmdir(path);
+ g_free(path);
+
+ return TRUE;
+}
+
+static gboolean sr_assembly_update_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ return sr_assembly_remove_fragment_backup(imsi, node, msg_id) &&
+ sr_assembly_add_fragment_backup(imsi, node, msg_id);
+}
+
void status_report_assembly_free(struct status_report_assembly *assembly)
{
g_hash_table_destroy(assembly->assembly_table);
@@ -2707,10 +2900,14 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
* not delete the node yet.
*/
if (pending) {
+ sr_assembly_update_fragment_backup(assembly->imsi, node,
+ *msg_id);
*msg_delivered = FALSE;
return update_history;
} else {
*msg_delivered = node->deliverable;
+ sr_assembly_remove_fragment_backup(assembly->imsi, node,
+ *msg_id);
g_hash_table_iter_remove(&iter);
@@ -2747,6 +2944,7 @@ void status_report_assembly_add_fragment(
unsigned int *id_table_key;
id_table = g_hash_table_lookup(assembly->assembly_table, to->address);
+
/* Create id_table and node */
if (id_table == NULL) {
id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
@@ -2775,6 +2973,9 @@ void status_report_assembly_add_fragment(
g_hash_table_insert(assembly->assembly_table,
assembly_table_key, id_table);
+
+ sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
+
return;
}
@@ -2793,12 +2994,14 @@ void status_report_assembly_add_fragment(
*id_table_key = msg_id;
g_hash_table_insert(id_table, id_table_key, node);
+ sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
return;
}
/* id_table and node both exists */
node->mrs[offset] |= bit;
node->expiration = expiration;
node->sent_mrs++;
+ sr_assembly_update_fragment_backup(assembly->imsi, node, msg_id);
}
void status_report_assembly_expire(struct status_report_assembly *assembly,
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [RFC PATCH 5/5] smsutil: save pending status reports to imsi file
2010-06-17 13:14 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Pasi Miettinen
@ 2010-06-21 21:07 ` Denis Kenzior
0 siblings, 0 replies; 5+ messages in thread
From: Denis Kenzior @ 2010-06-21 21:07 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 10143 bytes --]
Hi Pasi,
> ---
> src/smsutil.c | 205
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files
changed,
> 204 insertions(+), 1 deletions(-)
>
> diff --git a/src/smsutil.c b/src/smsutil.c
> index 3db3d28..a30a281 100644
> --- a/src/smsutil.c
> +++ b/src/smsutil.c
> @@ -45,6 +45,10 @@
> #define SMS_BACKUP_PATH_DIR SMS_BACKUP_PATH "/%s-%i-%i"
> #define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR "/%03i"
>
> +#define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
> +#define SMS_SR_BACKUP_PATH_DIR SMS_SR_BACKUP_PATH "/%s-%i-%i"
> +#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH_DIR "/%i"
> +
> #define SMS_ADDR_FMT "%24[0-9A-F]"
>
> static GSList *sms_assembly_add_fragment_backup(struct sms_assembly
> *assembly, @@ -2625,20 +2629,209 @@ void sms_assembly_expire(struct
> sms_assembly *assembly, time_t before) }
> }
>
> +static void sr_assembly_load_backup(GHashTable *assembly_table,
> + const char *imsi,
> + const struct dirent *addr_dir)
> +{
> + char *path;
> + struct dirent **ids;
> + struct sms_address *addr;
> + struct id_table_node *node;
> + GHashTable *id_table;
> + int len;
> + int r;
> + unsigned char buf[sizeof(node->mrs) + sizeof(node->total_mrs) +
> + sizeof(node->sent_mrs) + sizeof(node->deliverable)];
> + char *assembly_table_key;
> + unsigned int *id_table_key;
> + struct stat segment_stat;
> +
> + if (addr_dir->d_type != DT_DIR)
> + return;
> +
> + addr = g_new0(struct sms_address, 1);
Why are you mallocing addr here? Seems that this only makes the code more
complicated.
> +
> + if (sscanf(addr_dir->d_name, "%[0-9]-%i-%i",
> + addr->address, (int *) &addr->number_type,
> + (int *) &addr->numbering_plan) < 3) {
Lets make this consistent with sms assembly, please use SMS_ADDR_FMT. You're
also free to use sms_assembly utility functions if that helps you.
> + g_free(addr);
> + return;
> + }
> +
> + /* Go through different msg_ids. */
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", imsi,
> + addr_dir->d_name);
> + len = scandir(path, &ids, NULL, versionsort);
> +
> + g_free(path);
> +
> + if (len < 0) {
> + g_free(addr);
> + return;
> + }
> +
> + id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> + g_free, g_free);
> +
> + assembly_table_key = g_try_malloc(sizeof(addr->address));
> +
> + if (assembly_table_key == NULL) {
> + g_free(addr);
> + return;
> + }
> +
> + g_strlcpy(assembly_table_key, addr->address, sizeof(addr->address));
> + g_hash_table_insert(assembly_table, assembly_table_key, id_table);
> +
> + while (len--) {
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s/%s",
> + imsi, addr_dir->d_name, ids[len]->d_name);
> + r = read_file(buf, sizeof(buf), SMS_SR_BACKUP_PATH "/%s/%s",
> + imsi, addr_dir->d_name, ids[len]->d_name);
> +
> + if (r < 0) {
> + g_free(path);
> + g_free(ids[len]);
> + continue;
> + }
> +
> + r = stat(path, &segment_stat);
> +
> + if (r != 0) {
> + g_free(path);
> + g_free(ids[len]);
> + continue;
> + }
> + /* Gather the data for id_table node */
> + node = g_new0(struct id_table_node, 1);
> + node->to = *addr;
Please use memcpy for copying structures.
> + node->expiration = segment_stat.st_mtime;
> + memcpy(node->mrs, buf, sizeof(node->mrs));
> + memcpy(&node->total_mrs, buf + sizeof(node->mrs),
> + sizeof(node->total_mrs));
> + memcpy(&node->sent_mrs,
> + buf + sizeof(node->mrs) + sizeof(node->total_mrs),
> + sizeof(node->sent_mrs));
> +
> + memcpy(&node->deliverable, buf + sizeof(node->mrs) +
> + sizeof(node->total_mrs) + sizeof(node->sent_mrs),
> + sizeof(node->deliverable));
> + /* Node ready, create key and add them to the table */
> + id_table_key = g_new0(unsigned int, 1);
> + *id_table_key = atoi(ids[len]->d_name);
> +
> + g_hash_table_insert(id_table, id_table_key, node);
> +
> + g_free(path);
> + g_free(ids[len]);
> + }
> + g_free(addr);
> + g_free(ids);
> +}
> +
General comment here is that you're duplicating much of the code that inserts
the information into the data structure. You can play the same trick as
sms_assembly which uses a single function to do the heavy lifting, but takes
an argument whether to store this info on disk. Obviously during load you
don't want to re-save this information on disk. Then your load function
becomes much smaller and easier to understand.
> struct status_report_assembly *status_report_assembly_new(const char
> *imsi) {
> + char *path;
> + int len;
> + struct dirent **addresses;
> struct status_report_assembly *ret =
> g_new0(struct status_report_assembly, 1);
>
> ret->assembly_table = g_hash_table_new_full(g_str_hash, g_str_equal,
> g_free, (GDestroyNotify)g_hash_table_destroy);
>
> - if (imsi)
> + if (imsi) {
> ret->imsi = imsi;
>
> + /* Restore state from backup */
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH, imsi);
> + len = scandir(path, &addresses, NULL, alphasort);
> +
> + g_free(path);
> +
> + if (len < 0)
> + return ret;
There should be an empty line here
> + /* Go through different addresses. Each address can relate to
> + * 1-n msg_ids. Do not try to load . and .. directories.
> + */
> + g_free(addresses[0]);
> + g_free(addresses[1]);
This really isn't necessary, the '.' and '..' directories will not pass the
sscanf test above.
> +
> + while (2 < len--) {
> + sr_assembly_load_backup(ret->assembly_table, imsi,
> + addresses[len]);
> + g_free(addresses[len]);
> + }
Empty line here please
> + g_free(addresses);
> + }
And another empty line here. Please put an empty line after each block unless
it is at the end of the function.
> return ret;
> }
>
> +static gboolean sr_assembly_add_fragment_backup(const char *imsi,
> + const struct id_table_node *node,
> + unsigned int msg_id)
Lets be consistent with how sms assembly works. E.g. take the main assembly
object and return void. Just helps to read the code when it is consistent.
> +{
> + int len = sizeof(node->mrs) + sizeof(node->total_mrs) +
> + sizeof(node->sent_mrs) + sizeof(node->deliverable);
> + unsigned char buf[len];
> +
> + if (!imsi)
> + return FALSE;
> +
> + memcpy(buf, node->mrs, sizeof(node->mrs));
> +
> + memcpy(buf + sizeof(node->mrs), &node->total_mrs,
> + sizeof(node->total_mrs));
> +
> + memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs),
> + &node->sent_mrs, sizeof(node->sent_mrs));
> +
> + memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs) +
> + sizeof(node->sent_mrs), &node->deliverable, sizeof(node->deliverable));
> +
> + /* storagedir/%s/sms_sr/%s-%i-%i/%i */
> + if (write_file(buf, len, SMS_BACKUP_MODE, SMS_SR_BACKUP_PATH_FILE, imsi,
> + node->to.address, node->to.number_type,
> + node->to.numbering_plan, msg_id) != len)
> + return FALSE;
> +
> + return TRUE;
> +}
> +
> +static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
> + const struct id_table_node *node,
> + unsigned int msg_id)
Same consistency comment here. Take main assembly object and return void.
> +{
> + char *path;
> +
> + if (!imsi)
> + return FALSE;
> +
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, imsi, node->to.address,
> + node->to.number_type, node->to.numbering_plan,
> + msg_id);
> +
> + unlink(path);
> + g_free(path);
> +
> + path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, imsi, node->to.address,
> + node->to.number_type, node->to.numbering_plan);
> +
> + /* If the address does not have relating msg_ids anymore, remove it */
> + rmdir(path);
> + g_free(path);
> +
> + return TRUE;
> +}
> +
> +static gboolean sr_assembly_update_fragment_backup(const char *imsi,
> + const struct id_table_node *node,
> + unsigned int msg_id)
> +{
> + return sr_assembly_remove_fragment_backup(imsi, node, msg_id) &&
> + sr_assembly_add_fragment_backup(imsi, node, msg_id);
> +}
> +
Is this function really necessary? write_file overwrites the file, so simply
using add_fragment_backup seems sufficient.
> void status_report_assembly_free(struct status_report_assembly *assembly)
> {
> g_hash_table_destroy(assembly->assembly_table);
> @@ -2707,10 +2900,14 @@ gboolean status_report_assembly_report(struct
> status_report_assembly *assembly, * not delete the node yet.
> */
> if (pending) {
> + sr_assembly_update_fragment_backup(assembly->imsi, node,
> + *msg_id);
It seems to me fragment is the wrong name here. What about
'store_node_backup'?
> *msg_delivered = FALSE;
> return update_history;
> } else {
> *msg_delivered = node->deliverable;
> + sr_assembly_remove_fragment_backup(assembly->imsi, node,
> + *msg_id);
maybe 'remove_node_backup'
>
> g_hash_table_iter_remove(&iter);
>
> @@ -2747,6 +2944,7 @@ void status_report_assembly_add_fragment(
> unsigned int *id_table_key;
>
> id_table = g_hash_table_lookup(assembly->assembly_table, to->address);
> +
> /* Create id_table and node */
> if (id_table == NULL) {
> id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
> @@ -2775,6 +2973,9 @@ void status_report_assembly_add_fragment(
>
> g_hash_table_insert(assembly->assembly_table,
> assembly_table_key, id_table);
> +
> + sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
> +
> return;
> }
>
> @@ -2793,12 +2994,14 @@ void status_report_assembly_add_fragment(
> *id_table_key = msg_id;
> g_hash_table_insert(id_table, id_table_key, node);
>
> + sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
> return;
> }
> /* id_table and node both exists */
> node->mrs[offset] |= bit;
> node->expiration = expiration;
> node->sent_mrs++;
> + sr_assembly_update_fragment_backup(assembly->imsi, node, msg_id);
> }
>
> void status_report_assembly_expire(struct status_report_assembly
> *assembly,
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH 4/5] sms: Status report notify
@ 2010-06-16 14:08 Pasi Miettinen
2010-06-16 14:08 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Pasi Miettinen
0 siblings, 1 reply; 5+ messages in thread
From: Pasi Miettinen @ 2010-06-16 14:08 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]
---
src/sms.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 70 insertions(+), 1 deletions(-)
diff --git a/src/sms.c b/src/sms.c
index bf6d261..e6a2dca 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -68,6 +68,8 @@ struct ofono_sms {
void *driver_data;
struct ofono_atom *atom;
ofono_bool_t use_delivery_reports;
+ struct status_report_assembly *sr_assembly;
+
};
struct pending_pdu {
@@ -83,6 +85,8 @@ struct tx_queue_entry {
unsigned int msg_id;
unsigned int retry;
DBusMessage *msg;
+ gboolean status_report;
+ struct sms_address receiver;
};
static void set_sca(struct ofono_sms *sms,
@@ -331,6 +335,13 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
entry->cur_pdu += 1;
entry->retry = 0;
+ if (entry->status_report)
+ status_report_assembly_add_fragment(sms->sr_assembly,
+ entry->msg_id,
+ &entry->receiver,
+ mr, time(NULL),
+ entry->num_pdus);
+
if (entry->cur_pdu < entry->num_pdus) {
sms->tx_source = g_timeout_add(0, tx_next, sms);
return;
@@ -462,6 +473,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
set_ref_and_to(msg_list, sms->ref, ref_offset, to);
entry = create_tx_queue_entry(msg_list);
+ sms_address_from_string(&entry->receiver, to);
+
g_slist_foreach(msg_list, (GFunc)g_free, NULL);
g_slist_free(msg_list);
@@ -474,6 +487,7 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
entry->msg = dbus_message_ref(msg);
entry->msg_id = sms->next_msg_id++;
+ entry->status_report = sms->use_delivery_reports;
g_queue_push_tail(sms->txq, entry);
@@ -718,6 +732,30 @@ static void handle_deliver(struct ofono_sms *sms, const struct sms *incoming)
g_slist_free(l);
}
+static void handle_sms_status_report(struct ofono_sms *sms,
+ const struct sms *incoming)
+{
+ gboolean delivered;
+ unsigned int msg_id;
+ gboolean update_history;
+ struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
+
+ update_history = status_report_assembly_report(sms->sr_assembly,
+ incoming, &msg_id, &delivered);
+
+ if (update_history) {
+
+ if (delivered)
+ __ofono_history_sms_send_status(modem, msg_id,
+ time(NULL), OFONO_HISTORY_SMS_STATUS_DELIVERED);
+ else
+ __ofono_history_sms_send_status(modem, msg_id,
+ time(NULL),
+ OFONO_HISTORY_SMS_STATUS_DELIVER_FAILED);
+ }
+}
+
+
static inline gboolean handle_mwi(struct ofono_sms *sms, struct sms *s)
{
gboolean discard;
@@ -849,7 +887,30 @@ out:
void ofono_sms_status_notify(struct ofono_sms *sms, unsigned char *pdu,
int len, int tpdu_len)
{
- ofono_error("SMS Status-Report not yet handled");
+ struct sms s;
+ enum sms_class cls;
+
+ if (!sms_decode(pdu, len, FALSE, tpdu_len, &s)) {
+ ofono_error("Unable to decode PDU");
+ return;
+ }
+
+ if (s.type != SMS_TYPE_STATUS_REPORT) {
+ ofono_error("Expecting a STATUS REPORT pdu");
+ return;
+ }
+
+ if (s.status_report.srq) {
+ ofono_error("Waiting an answer to SMS-SUBMIT, not SMS-COMMAND");
+ return;
+ }
+
+ if (!sms_dcs_decode(s.deliver.dcs, &cls, NULL, NULL, NULL)) {
+ ofono_error("Unknown / Reserved DCS. Ignoring");
+ return;
+ }
+
+ handle_sms_status_report(sms, &s);
}
int ofono_sms_driver_register(const struct ofono_sms_driver *d)
@@ -932,6 +993,11 @@ static void sms_remove(struct ofono_atom *atom)
sms->settings = NULL;
}
+ if (sms->sr_assembly) {
+ status_report_assembly_free(sms->sr_assembly);
+ sms->sr_assembly = NULL;
+ }
+
g_free(sms);
}
@@ -1069,9 +1135,12 @@ void ofono_sms_register(struct ofono_sms *sms)
imsi = ofono_sim_get_imsi(sms->sim);
sms->assembly = sms_assembly_new(imsi);
+ sms->sr_assembly = status_report_assembly_new(imsi);
+
sms_load_settings(sms, imsi);
} else {
sms->assembly = sms_assembly_new(NULL);
+ sms->sr_assembly = status_report_assembly_new(NULL);
}
__ofono_atom_register(sms->atom, sms_unregister);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC PATCH 5/5] smsutil: save pending status reports to imsi file
2010-06-16 14:08 [RFC PATCH 4/5] sms: Status report notify Pasi Miettinen
@ 2010-06-16 14:08 ` Pasi Miettinen
0 siblings, 0 replies; 5+ messages in thread
From: Pasi Miettinen @ 2010-06-16 14:08 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8069 bytes --]
---
src/smsutil.c | 205 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 204 insertions(+), 1 deletions(-)
diff --git a/src/smsutil.c b/src/smsutil.c
index 3db3d28..a30a281 100644
--- a/src/smsutil.c
+++ b/src/smsutil.c
@@ -45,6 +45,10 @@
#define SMS_BACKUP_PATH_DIR SMS_BACKUP_PATH "/%s-%i-%i"
#define SMS_BACKUP_PATH_FILE SMS_BACKUP_PATH_DIR "/%03i"
+#define SMS_SR_BACKUP_PATH STORAGEDIR "/%s/sms_sr"
+#define SMS_SR_BACKUP_PATH_DIR SMS_SR_BACKUP_PATH "/%s-%i-%i"
+#define SMS_SR_BACKUP_PATH_FILE SMS_SR_BACKUP_PATH_DIR "/%i"
+
#define SMS_ADDR_FMT "%24[0-9A-F]"
static GSList *sms_assembly_add_fragment_backup(struct sms_assembly *assembly,
@@ -2625,20 +2629,209 @@ void sms_assembly_expire(struct sms_assembly *assembly, time_t before)
}
}
+static void sr_assembly_load_backup(GHashTable *assembly_table,
+ const char *imsi,
+ const struct dirent *addr_dir)
+{
+ char *path;
+ struct dirent **ids;
+ struct sms_address *addr;
+ struct id_table_node *node;
+ GHashTable *id_table;
+ int len;
+ int r;
+ unsigned char buf[sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs) + sizeof(node->deliverable)];
+ char *assembly_table_key;
+ unsigned int *id_table_key;
+ struct stat segment_stat;
+
+ if (addr_dir->d_type != DT_DIR)
+ return;
+
+ addr = g_new0(struct sms_address, 1);
+
+ if (sscanf(addr_dir->d_name, "%[0-9]-%i-%i",
+ addr->address, (int *) &addr->number_type,
+ (int *) &addr->numbering_plan) < 3) {
+ g_free(addr);
+ return;
+ }
+
+ /* Go through different msg_ids. */
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s", imsi,
+ addr_dir->d_name);
+ len = scandir(path, &ids, NULL, versionsort);
+
+ g_free(path);
+
+ if (len < 0) {
+ g_free(addr);
+ return;
+ }
+
+ id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
+ g_free, g_free);
+
+ assembly_table_key = g_try_malloc(sizeof(addr->address));
+
+ if (assembly_table_key == NULL) {
+ g_free(addr);
+ return;
+ }
+
+ g_strlcpy(assembly_table_key, addr->address, sizeof(addr->address));
+ g_hash_table_insert(assembly_table, assembly_table_key, id_table);
+
+ while (len--) {
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH "/%s/%s",
+ imsi, addr_dir->d_name, ids[len]->d_name);
+ r = read_file(buf, sizeof(buf), SMS_SR_BACKUP_PATH "/%s/%s",
+ imsi, addr_dir->d_name, ids[len]->d_name);
+
+ if (r < 0) {
+ g_free(path);
+ g_free(ids[len]);
+ continue;
+ }
+
+ r = stat(path, &segment_stat);
+
+ if (r != 0) {
+ g_free(path);
+ g_free(ids[len]);
+ continue;
+ }
+ /* Gather the data for id_table node */
+ node = g_new0(struct id_table_node, 1);
+ node->to = *addr;
+ node->expiration = segment_stat.st_mtime;
+ memcpy(node->mrs, buf, sizeof(node->mrs));
+ memcpy(&node->total_mrs, buf + sizeof(node->mrs),
+ sizeof(node->total_mrs));
+ memcpy(&node->sent_mrs,
+ buf + sizeof(node->mrs) + sizeof(node->total_mrs),
+ sizeof(node->sent_mrs));
+
+ memcpy(&node->deliverable, buf + sizeof(node->mrs) +
+ sizeof(node->total_mrs) + sizeof(node->sent_mrs),
+ sizeof(node->deliverable));
+ /* Node ready, create key and add them to the table */
+ id_table_key = g_new0(unsigned int, 1);
+ *id_table_key = atoi(ids[len]->d_name);
+
+ g_hash_table_insert(id_table, id_table_key, node);
+
+ g_free(path);
+ g_free(ids[len]);
+ }
+ g_free(addr);
+ g_free(ids);
+}
+
struct status_report_assembly *status_report_assembly_new(const char *imsi)
{
+ char *path;
+ int len;
+ struct dirent **addresses;
struct status_report_assembly *ret =
g_new0(struct status_report_assembly, 1);
ret->assembly_table = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, (GDestroyNotify)g_hash_table_destroy);
- if (imsi)
+ if (imsi) {
ret->imsi = imsi;
+ /* Restore state from backup */
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH, imsi);
+ len = scandir(path, &addresses, NULL, alphasort);
+
+ g_free(path);
+
+ if (len < 0)
+ return ret;
+ /* Go through different addresses. Each address can relate to
+ * 1-n msg_ids. Do not try to load . and .. directories.
+ */
+ g_free(addresses[0]);
+ g_free(addresses[1]);
+
+ while (2 < len--) {
+ sr_assembly_load_backup(ret->assembly_table, imsi,
+ addresses[len]);
+ g_free(addresses[len]);
+ }
+ g_free(addresses);
+ }
return ret;
}
+static gboolean sr_assembly_add_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ int len = sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs) + sizeof(node->deliverable);
+ unsigned char buf[len];
+
+ if (!imsi)
+ return FALSE;
+
+ memcpy(buf, node->mrs, sizeof(node->mrs));
+
+ memcpy(buf + sizeof(node->mrs), &node->total_mrs,
+ sizeof(node->total_mrs));
+
+ memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs),
+ &node->sent_mrs, sizeof(node->sent_mrs));
+
+ memcpy(buf + sizeof(node->mrs) + sizeof(node->total_mrs) +
+ sizeof(node->sent_mrs), &node->deliverable, sizeof(node->deliverable));
+
+ /* storagedir/%s/sms_sr/%s-%i-%i/%i */
+ if (write_file(buf, len, SMS_BACKUP_MODE, SMS_SR_BACKUP_PATH_FILE, imsi,
+ node->to.address, node->to.number_type,
+ node->to.numbering_plan, msg_id) != len)
+ return FALSE;
+
+ return TRUE;
+}
+
+static gboolean sr_assembly_remove_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ char *path;
+
+ if (!imsi)
+ return FALSE;
+
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH_FILE, imsi, node->to.address,
+ node->to.number_type, node->to.numbering_plan,
+ msg_id);
+
+ unlink(path);
+ g_free(path);
+
+ path = g_strdup_printf(SMS_SR_BACKUP_PATH_DIR, imsi, node->to.address,
+ node->to.number_type, node->to.numbering_plan);
+
+ /* If the address does not have relating msg_ids anymore, remove it */
+ rmdir(path);
+ g_free(path);
+
+ return TRUE;
+}
+
+static gboolean sr_assembly_update_fragment_backup(const char *imsi,
+ const struct id_table_node *node,
+ unsigned int msg_id)
+{
+ return sr_assembly_remove_fragment_backup(imsi, node, msg_id) &&
+ sr_assembly_add_fragment_backup(imsi, node, msg_id);
+}
+
void status_report_assembly_free(struct status_report_assembly *assembly)
{
g_hash_table_destroy(assembly->assembly_table);
@@ -2707,10 +2900,14 @@ gboolean status_report_assembly_report(struct status_report_assembly *assembly,
* not delete the node yet.
*/
if (pending) {
+ sr_assembly_update_fragment_backup(assembly->imsi, node,
+ *msg_id);
*msg_delivered = FALSE;
return update_history;
} else {
*msg_delivered = node->deliverable;
+ sr_assembly_remove_fragment_backup(assembly->imsi, node,
+ *msg_id);
g_hash_table_iter_remove(&iter);
@@ -2747,6 +2944,7 @@ void status_report_assembly_add_fragment(
unsigned int *id_table_key;
id_table = g_hash_table_lookup(assembly->assembly_table, to->address);
+
/* Create id_table and node */
if (id_table == NULL) {
id_table = g_hash_table_new_full(g_int_hash, g_int_equal,
@@ -2775,6 +2973,9 @@ void status_report_assembly_add_fragment(
g_hash_table_insert(assembly->assembly_table,
assembly_table_key, id_table);
+
+ sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
+
return;
}
@@ -2793,12 +2994,14 @@ void status_report_assembly_add_fragment(
*id_table_key = msg_id;
g_hash_table_insert(id_table, id_table_key, node);
+ sr_assembly_add_fragment_backup(assembly->imsi, node, msg_id);
return;
}
/* id_table and node both exists */
node->mrs[offset] |= bit;
node->expiration = expiration;
node->sent_mrs++;
+ sr_assembly_update_fragment_backup(assembly->imsi, node, msg_id);
}
void status_report_assembly_expire(struct status_report_assembly *assembly,
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-08-12 17:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <FEE7A63B35E92D4F93D1BB82DE254AE5021987@JKLMAIL02.ixonos.local>
2010-08-10 16:25 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Tikander Petteri
2010-08-12 17:24 ` Denis Kenzior
2010-06-17 13:14 [RFC PATCH 4/5] sms: Status report notify Pasi Miettinen
2010-06-17 13:14 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Pasi Miettinen
2010-06-21 21:07 ` Denis Kenzior
-- strict thread matches above, loose matches on Subject: below --
2010-06-16 14:08 [RFC PATCH 4/5] sms: Status report notify Pasi Miettinen
2010-06-16 14:08 ` [RFC PATCH 5/5] smsutil: save pending status reports to imsi file Pasi Miettinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox