Hi Inaky, On 08/03/2010 06:50 PM, Inaky Perez-Gonzalez wrote: > From: Inaky Perez-Gonzalez > > This patch removes the sequential SMS message identification gig and > replaces it with a 16-bit hash cookie based off message contents. > > FIXME: [incomplete] need to figure out how to do so that identical > messages sent to different or the same numbers also have a different > ID. > --- > src/ofono.h | 9 +++++---- > src/sms.c | 55 +++++++++++++++++++++++-------------------------------- > src/stk.c | 5 +++-- > 3 files changed, 31 insertions(+), 38 deletions(-) > > diff --git a/src/ofono.h b/src/ofono.h > index aaa01d9..b73797b 100644 > --- a/src/ofono.h > +++ b/src/ofono.h > @@ -185,10 +185,11 @@ enum ofono_sms_submit_flag { > > typedef void (*ofono_sms_txq_submit_cb_t)(gboolean ok, void *data); > > -unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, > - unsigned int flags, > - ofono_sms_txq_submit_cb_t cb, > - void *data, ofono_destroy_func destroy); > +struct tx_queue_entry *__ofono_sms_txq_submit( > + struct ofono_sms *sms, GSList *list, > + unsigned int flags, unsigned ref, > + ofono_sms_txq_submit_cb_t cb, > + void *data, ofono_destroy_func destroy); As I said before, please don't mess with this function. > > #include > #include > diff --git a/src/sms.c b/src/sms.c > index 35364db..39d4a32 100644 > --- a/src/sms.c > +++ b/src/sms.c > @@ -55,7 +55,6 @@ struct ofono_sms { > DBusMessage *pending; > struct ofono_phone_number sca; > struct sms_assembly *assembly; > - unsigned int next_msg_id; > guint ref; > GQueue *txq; > gint tx_source; > @@ -589,7 +588,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, > int ref_offset; > struct ofono_modem *modem; > unsigned int flags; > - unsigned int msg_id; > + guint16 msg_id; > + struct tx_queue_entry *sms_msg; > > if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to, > DBUS_TYPE_STRING, &text, > @@ -605,24 +605,19 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg, > if (!msg_list) > return __ofono_error_invalid_format(msg); > > - set_ref_and_to(msg_list, sms->ref, ref_offset, to); > - DBG("ref: %d, offset: %d", sms->ref, ref_offset); > - > - if (ref_offset != 0) { > - if (sms->ref == 65536) > - sms->ref = 1; > - else > - sms->ref = sms->ref + 1; > - } > + msg_id = sms_uuid_from_pdu_list(msg_list); You're calculating the ID before setting the ref and to? That is pretty silly ;) > + set_ref_and_to(msg_list, msg_id, ref_offset, to); > + DBG("SMS ID: 0x%04x", msg_id); > > flags = OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY; > flags |= OFONO_SMS_SUBMIT_FLAG_RETRY; > if (sms->use_delivery_reports) > flags |= OFONO_SMS_SUBMIT_FLAG_REQUEST_SR; > > - msg_id = __ofono_sms_txq_submit(sms, msg_list, flags, send_message_cb, > - dbus_message_ref(msg), > - send_message_destroy); > + sms_msg = __ofono_sms_txq_submit(sms, msg_list, flags, msg_id, > + send_message_cb, > + dbus_message_ref(msg), > + send_message_destroy); Make the ID calculation happen inside tx_queue_entry_new. That function already converts all the SMS objects into a PDU. So simply hashing the PDUs there would be very easy. > > g_slist_foreach(msg_list, (GFunc)g_free, NULL); > g_slist_free(msg_list); > @@ -659,7 +654,7 @@ static void dispatch_app_datagram(struct ofono_sms *sms, int dst, int src, > } > > static void dispatch_text_message(struct ofono_sms *sms, > - const char *message, > + const char *message, guint16 msg_id, > enum sms_class cls, > const struct sms_address *addr, > const struct sms_scts *scts) > @@ -717,11 +712,9 @@ static void dispatch_text_message(struct ofono_sms *sms, > > g_dbus_send_message(conn, signal); > > - if (cls != SMS_CLASS_0) { > - __ofono_history_sms_received(modem, sms->next_msg_id, str, > + if (cls != SMS_CLASS_0) > + __ofono_history_sms_received(modem, msg_id, str, > &remote, &local, message); > - sms->next_msg_id += 1; > - } > } > > static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list) > @@ -732,6 +725,7 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list) > enum sms_class cls; > int srcport = -1; > int dstport = -1; > + guint16 msg_id; > > if (sms_list == NULL) > return; > @@ -749,6 +743,7 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list) > * used, the addresses are the same across all segments. > */ > > + msg_id = sms_uuid_from_pdu_list(sms_list); > for (l = sms_list; l; l = l->next) { > guint8 dcs; > gboolean comp = FALSE; > @@ -825,8 +820,8 @@ static void sms_dispatch(struct ofono_sms *sms, GSList *sms_list) > > s = sms_list->data; > > - dispatch_text_message(sms, message, cls, &s->deliver.oaddr, > - &s->deliver.scts); > + dispatch_text_message(sms, message, msg_id, cls, > + &s->deliver.oaddr, &s->deliver.scts); > g_free(message); > } > } > @@ -1104,8 +1099,6 @@ static void sms_remove(struct ofono_atom *atom) > > if (sms->settings) { > g_key_file_set_integer(sms->settings, SETTINGS_GROUP, > - "NextMessageId", sms->next_msg_id); > - g_key_file_set_integer(sms->settings, SETTINGS_GROUP, > "NextReference", sms->ref); > g_key_file_set_boolean(sms->settings, SETTINGS_GROUP, > "UseDeliveryReports", > @@ -1201,9 +1194,6 @@ static void sms_load_settings(struct ofono_sms *sms, const char *imsi) > > sms->imsi = g_strdup(imsi); > > - sms->next_msg_id = g_key_file_get_integer(sms->settings, SETTINGS_GROUP, > - "NextMessageId", NULL); > - > sms->ref = g_key_file_get_integer(sms->settings, SETTINGS_GROUP, > "NextReference", NULL); > if (sms->ref >= 65536) > @@ -1306,10 +1296,11 @@ void *ofono_sms_get_data(struct ofono_sms *sms) > return sms->driver_data; > } > > -unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, > - unsigned int flags, > - ofono_sms_txq_submit_cb_t cb, > - void *data, ofono_destroy_func destroy) > +struct tx_queue_entry *__ofono_sms_txq_submit( > + struct ofono_sms *sms, GSList *list, > + unsigned int flags, unsigned msg_id, > + ofono_sms_txq_submit_cb_t cb, > + void *data, ofono_destroy_func destroy) > { > struct tx_queue_entry *entry = tx_queue_entry_new(list); > > @@ -1320,7 +1311,7 @@ unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, > sizeof(entry->receiver)); > } > > - entry->msg_id = sms->next_msg_id++; > + entry->msg_id = msg_id; > entry->flags = flags; > entry->cb = cb; > entry->data = data; > @@ -1331,5 +1322,5 @@ unsigned int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list, > if (g_queue_get_length(sms->txq) == 1) > sms->tx_source = g_timeout_add(0, tx_next, sms); > > - return entry->msg_id; > + return entry; > } > diff --git a/src/stk.c b/src/stk.c > index 556dc68..620811b 100644 > --- a/src/stk.c > +++ b/src/stk.c > @@ -323,6 +323,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd, > struct ofono_atom *sms_atom; > struct ofono_sms *sms; > GSList msg_list; > + guint16 msg_id; > > sms_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SMS); > > @@ -338,8 +339,8 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd, > > msg_list.data = (void *) &cmd->send_sms.gsm_sms; > msg_list.next = NULL; > - > - __ofono_sms_txq_submit(sms, &msg_list, 0, send_sms_submit_cb, > + msg_id = sms_uuid_from_pdu_list(&msg_list); > + __ofono_sms_txq_submit(sms, &msg_list, 0, msg_id, send_sms_submit_cb, > stk->sms_submit_req, g_free); > > stk->cancel_cmd = send_sms_cancel; Regards, -Denis