sms: Refactor ofono_sms_submit introduced by stk

Simplify the logic and get rid of duplicated code

3 files changed, 108 insertions, 96 deletions

diff --git a/src/ofono.h b/src/ofono.h
index 94e27157..aaa01d93 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -176,8 +176,19 @@ void __ofono_atom_free(struct ofono_atom *atom);
 #include <ofono/sms.h>
 
 struct sms;
-void __ofono_sms_submit(struct ofono_sms *sms, const struct sms *msg,
-			ofono_sms_submit_cb_t cb, void *data);
+
+enum ofono_sms_submit_flag {
+	OFONO_SMS_SUBMIT_FLAG_REQUEST_SR =	0x1,
+	OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY =	0x2,
+	OFONO_SMS_SUBMIT_FLAG_RETRY =		0x4,
+};
+
+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);
 
 #include <ofono/sim.h>
 #include <ofono/stk.h>
diff --git a/src/sms.c b/src/sms.c
index 4b1deb42..71e24c1a 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -82,13 +82,13 @@ struct tx_queue_entry {
 	struct pending_pdu *pdus;
 	unsigned char num_pdus;
 	unsigned char cur_pdu;
+	struct sms_address receiver;
 	unsigned int msg_id;
 	unsigned int retry;
-	DBusMessage *msg;
-	gboolean status_report;
-	struct sms_address receiver;
-	ofono_sms_submit_cb_t cb;
+	unsigned int flags;
+	ofono_sms_txq_submit_cb_t cb;
 	void *data;
+	ofono_destroy_func destroy;
 };
 
 static const char *sms_bearer_to_string(int bearer)
@@ -411,12 +411,13 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 	struct ofono_sms *sms = data;
 	struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
 	struct tx_queue_entry *entry = g_queue_peek_head(sms->txq);
+	gboolean ok = error->type == OFONO_ERROR_TYPE_NO_ERROR;
 
 	DBG("tx_finished");
 
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		if (entry->cb)
-			goto callback;
+	if (ok == FALSE) {
+		if (!(entry->flags & OFONO_SMS_SUBMIT_FLAG_RETRY))
+			goto next_q;
 
 		entry->retry += 1;
 
@@ -429,30 +430,13 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 		}
 
 		DBG("Max retries reached, giving up");
-
-		entry = g_queue_pop_head(sms->txq);
-		__ofono_dbus_pending_reply(&entry->msg,
-					__ofono_error_failed(entry->msg));
-
-		__ofono_history_sms_send_status(modem, entry->msg_id,
-					time(NULL),
-					OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED);
-
-		g_free(entry->pdus);
-		g_free(entry);
-
-		if (g_queue_peek_head(sms->txq)) {
-			DBG("Previous send failed, scheduling next");
-			sms->tx_source = g_timeout_add(0, tx_next, sms);
-		}
-
-		return;
+		goto next_q;
 	}
 
 	entry->cur_pdu += 1;
 	entry->retry = 0;
 
-	if (entry->status_report)
+	if (entry->flags & OFONO_SMS_SUBMIT_FLAG_REQUEST_SR)
 		status_report_assembly_add_fragment(sms->sr_assembly,
 							entry->msg_id,
 							&entry->receiver,
@@ -464,36 +448,34 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 		return;
 	}
 
-	if (entry->cb)
-		goto callback;
-
+next_q:
 	entry = g_queue_pop_head(sms->txq);
-	__ofono_dbus_pending_reply(&entry->msg,
-				dbus_message_new_method_return(entry->msg));
-	__ofono_history_sms_send_status(modem, entry->msg_id,
-					time(NULL),
-					OFONO_HISTORY_SMS_STATUS_SUBMITTED);
 
-	g_free(entry->pdus);
-	g_free(entry);
+	if (entry->cb)
+		entry->cb(ok, entry->data);
 
-	if (g_queue_peek_head(sms->txq)) {
-		DBG("Scheduling next");
-		sms->tx_source = g_timeout_add(0, tx_next, sms);
-	}
+	if (entry->flags & OFONO_SMS_SUBMIT_FLAG_RECORD_HISTORY) {
+		enum ofono_history_sms_status hs;
 
-	return;
+		if (ok)
+			hs = OFONO_HISTORY_SMS_STATUS_SUBMITTED;
+		else
+			hs = OFONO_HISTORY_SMS_STATUS_SUBMIT_FAILED;
 
-callback:
-	entry = g_queue_pop_head(sms->txq);
+		__ofono_history_sms_send_status(modem, entry->msg_id,
+						time(NULL), hs);
+	}
 
-	entry->cb(error, mr, entry->data);
+	if (entry->destroy)
+		entry->destroy(entry->data);
 
 	g_free(entry->pdus);
 	g_free(entry);
 
-	if (g_queue_peek_head(sms->txq))
+	if (g_queue_peek_head(sms->txq)) {
+		DBG("Scheduling next");
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
+	}
 }
 
 static gboolean tx_next(gpointer user_data)
@@ -563,6 +545,27 @@ static struct tx_queue_entry *create_tx_queue_entry(GSList *msg_list)
 	return entry;
 }
 
+static void send_message_cb(gboolean ok, void *data)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *msg = data;
+	DBusMessage *reply;
+
+	if (ok)
+		reply = dbus_message_new_method_return(msg);
+	else
+		reply = __ofono_error_failed(msg);
+
+	g_dbus_send_message(conn, reply);
+}
+
+static void send_message_destroy(void *data)
+{
+	DBusMessage *msg = data;
+
+	dbus_message_unref(msg);
+}
+
 /*
  * Pre-process a SMS text message and deliver it [D-Bus SendMessage()]
  *
@@ -584,8 +587,9 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
 	const char *text;
 	GSList *msg_list;
 	int ref_offset;
-	struct tx_queue_entry *entry;
 	struct ofono_modem *modem;
+	unsigned int flags;
+	unsigned int msg_id;
 
 	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &to,
 					DBUS_TYPE_STRING, &text,
@@ -601,15 +605,8 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
 	if (!msg_list)
 		return __ofono_error_invalid_format(msg);
 
-	DBG("ref: %d, offset: %d", sms->ref, ref_offset);
-
 	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);
+	DBG("ref: %d, offset: %d", sms->ref, ref_offset);
 
 	if (ref_offset != 0) {
 		if (sms->ref == 65536)
@@ -618,18 +615,20 @@ static DBusMessage *sms_send_message(DBusConnection *conn, DBusMessage *msg,
 			sms->ref = sms->ref + 1;
 	}
 
-	entry->msg = dbus_message_ref(msg);
-	entry->msg_id = sms->next_msg_id++;
-	entry->status_report = sms->use_delivery_reports;
+	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;
 
-	g_queue_push_tail(sms->txq, entry);
+	msg_id = __ofono_sms_txq_submit(sms, msg_list, flags, send_message_cb,
+						dbus_message_ref(msg),
+						send_message_destroy);
 
-	modem = __ofono_atom_get_modem(sms->atom);
-	__ofono_history_sms_send_pending(modem, entry->msg_id, to,
-						time(NULL), text);
+	g_slist_foreach(msg_list, (GFunc)g_free, NULL);
+	g_slist_free(msg_list);
 
-	if (g_queue_get_length(sms->txq) == 1)
-		sms->tx_source = g_timeout_add(0, tx_next, sms);
+	modem = __ofono_atom_get_modem(sms->atom);
+	__ofono_history_sms_send_pending(modem, msg_id, to, time(NULL), text);
 
 	return NULL;
 }
@@ -1307,20 +1306,30 @@ void *ofono_sms_get_data(struct ofono_sms *sms)
 	return sms->driver_data;
 }
 
-void __ofono_sms_submit(struct ofono_sms *sms, const struct sms *msg,
-			ofono_sms_submit_cb_t cb, 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)
 {
-	GSList msg_list = {
-		.data = (void *) msg,
-		.next = NULL,
-	};
-	struct tx_queue_entry *entry = create_tx_queue_entry(&msg_list);
+	struct tx_queue_entry *entry = create_tx_queue_entry(list);
 
+	if (flags & OFONO_SMS_SUBMIT_FLAG_REQUEST_SR) {
+		struct sms *head = list->data;
+
+		memcpy(&entry->receiver, &head->submit.daddr,
+				sizeof(entry->receiver));
+	}
+
+	entry->msg_id = sms->next_msg_id++;
+	entry->flags = flags;
 	entry->cb = cb;
 	entry->data = data;
+	entry->destroy = destroy;
 
 	g_queue_push_tail(sms->txq, entry);
 
 	if (g_queue_get_length(sms->txq) == 1)
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
+
+	return entry->msg_id;
 }
diff --git a/src/stk.c b/src/stk.c
index 30ae98fd..7cdad5bf 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -286,42 +286,32 @@ static void send_sms_cancel(struct ofono_stk *stk)
 	stk_alpha_id_unset(stk);
 }
 
-static void send_sms_submit_cb(const struct ofono_error *error, int mr,
-				void *data)
+static void send_sms_submit_cb(gboolean ok, void *data)
 {
-	struct stk_response rsp;
 	struct sms_submit_req *req = data;
 	struct ofono_stk *stk = req->stk;
 	struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+	struct stk_response rsp;
 
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
-		ofono_debug("SMS submission returned errors: %s",
-				telephony_error_to_str(error));
-	else
-		ofono_debug("SMS submission successful");
+	ofono_debug("SMS submission %s", ok ? "successful" : "failed");
 
 	if (req->cancelled) {
 		ofono_debug("Received an SMS submitted callback after the "
 				"proactive command was cancelled");
-		goto out;
+		return;
 	}
 
 	memset(&rsp, 0, sizeof(rsp));
 
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+	if (ok == FALSE)
 		rsp.result.type = STK_RESULT_TYPE_NETWORK_UNAVAILABLE;
 
 	if (stk_respond(stk, &rsp, stk_command_cb))
 		stk_command_cb(&failure, stk);
 
-	if (!stk->pending_cmd->send_sms.alpha_id ||
-			!stk->pending_cmd->send_sms.alpha_id[0])
-		goto out;
-
-	stk_alpha_id_unset(stk);
-
-out:
-	g_free(req);
+	if (stk->pending_cmd->send_sms.alpha_id &&
+			stk->pending_cmd->send_sms.alpha_id[0])
+		stk_alpha_id_unset(stk);
 }
 
 static gboolean handle_command_send_sms(const struct stk_command *cmd,
@@ -331,6 +321,7 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
 	struct ofono_modem *modem = __ofono_atom_get_modem(stk->atom);
 	struct ofono_atom *sms_atom;
 	struct ofono_sms *sms;
+	GSList msg_list;
 
 	sms_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SMS);
 
@@ -344,15 +335,16 @@ static gboolean handle_command_send_sms(const struct stk_command *cmd,
 	stk->sms_submit_req = g_new0(struct sms_submit_req, 1);
 	stk->sms_submit_req->stk = stk;
 
-	__ofono_sms_submit(sms, &cmd->send_sms.gsm_sms,
-				send_sms_submit_cb, stk->sms_submit_req);
+	msg_list.data = (void *) &cmd->send_sms.gsm_sms;
+	msg_list.next = NULL;
 
-	stk->cancel_cmd = send_sms_cancel;
+	__ofono_sms_txq_submit(sms, &msg_list, 0, send_sms_submit_cb,
+				stk->sms_submit_req, g_free);
 
-	if (!cmd->send_sms.alpha_id || !cmd->send_sms.alpha_id[0])
-		return FALSE;
+	stk->cancel_cmd = send_sms_cancel;
 
-	stk_alpha_id_set(stk, cmd->send_sms.alpha_id);
+	if (cmd->send_sms.alpha_id && cmd->send_sms.alpha_id[0])
+		stk_alpha_id_set(stk, cmd->send_sms.alpha_id);
 
 	return FALSE;
 }