Refactor phonebook code

Use immediate mode reporting, which means that the individual CPBR
lines are reported up to the core immediately.  This has a couple
advantages:
  - We do not need to malloc/free a bunch of single variables
    and copy them over.  Helps performance.
  - The lines are not buffered up and given to the plugin in one
    shot, instead processing is performed piecemeal.  This helps
    with keeping memory consumption down to a minimum

3 files changed, 124 insertions, 181 deletions

diff --git a/drivers/atmodem/phonebook.c b/drivers/atmodem/phonebook.c
index 471e597e..82adf100 100644
--- a/drivers/atmodem/phonebook.c
+++ b/drivers/atmodem/phonebook.c
@@ -44,110 +44,66 @@
 static const char *none_prefix[] = { NULL };
 static const char *entries_prefix[] = { "+CPBR:", NULL };
 
-static void at_read_entries_cb(gboolean ok, GAtResult *result,
-				gpointer user_data)
+static void at_cpbr_notify(GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
-	ofono_phonebook_export_entries_t cb = cbd->cb;
-	struct ofono_error error;
 	GAtResultIter iter;
-	int num_entries_max = 0;
-	int num_entries_real;
-	int num;
-	struct ofono_phonebook_entry *entries;
-
-	decode_at_error(&error, g_at_result_final_response(result));
-	if (!ok) {
-		cb(&error, 0, NULL, cbd->data);
-		return;
-	}
-
 	g_at_result_iter_init(&iter, result);
 
-	while (g_at_result_iter_next(&iter, "+CPBR:"))
-		num_entries_max += 1;
-
-	entries = g_new(struct ofono_phonebook_entry, num_entries_max);
-
-	g_at_result_iter_init(&iter, result);
-	for (num_entries_real = 0; g_at_result_iter_next(&iter, "+CPBR:");) {
-		int index, type, hidden, adtype;
-		const char *number, *text, *group, *adnumber,
-			*secondtext, *email, *sip_uri, *tel_uri;
+	while (g_at_result_iter_next(&iter, "+CPBR:")) {
+		int index;
+		const char *number;
+		int type;
+		const char *text;
+		int hidden = 0;
+		const char *group = NULL;
+		const char *adnumber = NULL;
+		int adtype = -1;
+		const char *secondtext = NULL;
+		const char *email = NULL;
+		const char *sip_uri = NULL;
+		const char *tel_uri = NULL;
 
 		if (!g_at_result_iter_next_number(&iter, &index))
 			continue;
-		entries[num_entries_real].index = index;
 
 		if (!g_at_result_iter_next_string(&iter, &number))
 			continue;
-		entries[num_entries_real].number = g_strdup(number);
 
-		if (!g_at_result_iter_next_number(&iter, &type)) {
-			g_free(entries[num_entries_real].number);
+		if (!g_at_result_iter_next_number(&iter, &type))
 			continue;
-		}
-		entries[num_entries_real].type = type;
 
-		if (!g_at_result_iter_next_string(&iter, &text)) {
-			g_free(entries[num_entries_real].number);
+		if (!g_at_result_iter_next_string(&iter, &text))
 			continue;
-		}
-		entries[num_entries_real].text = g_strdup(text);
-
-		if (!g_at_result_iter_next_number(&iter, &hidden))
-			hidden = -1;
-		entries[num_entries_real].hidden = hidden;
-
-		if (!g_at_result_iter_next_string(&iter, &group))
-			group = NULL;
-		entries[num_entries_real].group = g_strdup(group);
-
-		if (!g_at_result_iter_next_string(&iter, &adnumber))
-			adnumber = NULL;
-		entries[num_entries_real].adnumber = g_strdup(adnumber);
-
-		if (!g_at_result_iter_next_number(&iter, &adtype))
-			adtype = -1;
-		entries[num_entries_real].adtype = adtype;
-
-		if (!g_at_result_iter_next_string(&iter, &secondtext))
-			secondtext = NULL;
-		entries[num_entries_real].secondtext = g_strdup(secondtext);
-
-		if (!g_at_result_iter_next_string(&iter, &email))
-			email = NULL;
-		entries[num_entries_real].email = g_strdup(email);
 
-		if (!g_at_result_iter_next_string(&iter, &sip_uri))
-			sip_uri = NULL;
-		entries[num_entries_real].sip_uri = g_strdup(sip_uri);
+		g_at_result_iter_next_number(&iter, &hidden);
+		g_at_result_iter_next_string(&iter, &group);
+		g_at_result_iter_next_string(&iter, &adnumber);
+		g_at_result_iter_next_number(&iter, &adtype);
+		g_at_result_iter_next_string(&iter, &secondtext);
+		g_at_result_iter_next_string(&iter, &email);
+		g_at_result_iter_next_string(&iter, &sip_uri);
+		g_at_result_iter_next_string(&iter, &tel_uri);
+
+		ofono_phonebook_entry(cbd->modem, index, number, type, text,
+					hidden, group, adnumber, adtype,
+					secondtext, email, sip_uri, tel_uri);
+	}
+}
 
-		if (!g_at_result_iter_next_string(&iter, &tel_uri))
-			tel_uri = NULL;
-		entries[num_entries_real].tel_uri = g_strdup(tel_uri);
+static void at_read_entries_cb(gboolean ok, GAtResult *result,
+				gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_generic_cb_t cb = cbd->cb;
+	struct ofono_error error;
 
-		num_entries_real++;
-	}
-	cb(&error, num_entries_real, entries, cbd->data);
-
-	for (num = 0; num < num_entries_real; num++) {
-		g_free(entries[num].number);
-		g_free(entries[num].text);
-		g_free(entries[num].group);
-		g_free(entries[num].adnumber);
-		g_free(entries[num].secondtext);
-		g_free(entries[num].email);
-		g_free(entries[num].sip_uri);
-		g_free(entries[num].tel_uri);
-	}
-	g_free(entries);
-	entries = NULL;
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
 }
 
 static void at_read_entries(struct ofono_modem *modem, int index_min,
-				int index_max,
-				ofono_phonebook_export_entries_t cb,
+				int index_max, ofono_generic_cb_t cb,
 				void *data)
 {
 	struct at_data *at = ofono_modem_userdata(modem);
@@ -158,8 +114,9 @@ static void at_read_entries(struct ofono_modem *modem, int index_min,
 		goto error;
 
 	sprintf(buf, "AT+CPBR=%d,%d", index_min, index_max);
-	if (g_at_chat_send(at->parser, buf, entries_prefix,
-				at_read_entries_cb, cbd, g_free) > 0)
+	if (g_at_chat_send_listing(at->parser, buf, entries_prefix,
+					at_cpbr_notify, at_read_entries_cb,
+					cbd, g_free) > 0)
 		return;
 
 error:
@@ -168,7 +125,7 @@ error:
 
 	{
 		DECLARE_FAILURE(error);
-		cb(&error, 0, NULL, data);
+		cb(&error, data);
 	}
 }
 
@@ -177,15 +134,16 @@ static void at_list_indices_cb(gboolean ok, GAtResult *result,
 {
 	struct cb_data *cbd = user_data;
 	struct ofono_modem *modem = cbd->modem;
-	ofono_phonebook_export_entries_t cb = cbd->cb;
-	struct ofono_error error;
+	ofono_generic_cb_t cb = cbd->cb;
 	GAtResultIter iter;
 	int index_min;
 	int index_max;
 
-	decode_at_error(&error, g_at_result_final_response(result));
 	if (!ok) {
-		cb(&error, 0, NULL, cbd->data);
+		struct ofono_error error;
+
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, cbd->data);
 		return;
 	}
 
@@ -217,8 +175,7 @@ fail:
 }
 
 static void at_list_indices(struct ofono_modem *modem,
-				ofono_phonebook_export_entries_t cb,
-				void *data)
+				ofono_generic_cb_t cb, void *data)
 {
 	struct at_data *at = ofono_modem_userdata(modem);
 	struct cb_data *cbd = cb_data_new(modem, cb, data);
@@ -236,7 +193,7 @@ error:
 
 	{
 		DECLARE_FAILURE(error);
-		cb(&error, 0, NULL, data);
+		cb(&error, data);
 	}
 }
 
@@ -245,19 +202,20 @@ static void at_select_storage_cb(gboolean ok, GAtResult *result,
 {
 	struct cb_data *cbd = user_data;
 	struct ofono_modem *modem = cbd->modem;
-	ofono_phonebook_export_entries_t cb = cbd->cb;
-	struct ofono_error error;
+	ofono_generic_cb_t cb = cbd->cb;
 
-	decode_at_error(&error, g_at_result_final_response(result));
 	if (!ok) {
-		cb(&error, 0, NULL, cbd->data);
+		struct ofono_error error;
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, cbd->data);
 		return;
-	} else
-		at_list_indices(modem, cb, modem);
+	}
+
+	at_list_indices(modem, cb, modem);
 }
 
 static void at_export_entries(struct ofono_modem *modem, const char *storage,
-			ofono_phonebook_export_entries_t cb, void *data)
+				ofono_generic_cb_t cb, void *data)
 {
 	struct at_data *at = ofono_modem_userdata(modem);
 	struct cb_data *cbd = cb_data_new(modem, cb, data);
@@ -277,7 +235,7 @@ error:
 
 	{
 		DECLARE_FAILURE(error);
-		cb(&error, 0, NULL, data);
+		cb(&error, data);
 	}
 }
 
diff --git a/src/driver.h b/src/driver.h
index 9cf3c01c..3a690f0f 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -102,21 +102,6 @@ struct ofono_own_number {
 	int itc;
 };
 
-struct ofono_phonebook_entry {
-	int index;
-	char *number;
-	int type;
-	char *text;
-	int hidden;
-	char *group;
-	char *adnumber;
-	int adtype;
-	char *secondtext;
-	char *email;
-	char *sip_uri;
-	char *tel_uri;
-};
-
 /* Notification functions, the integer values here should map to
  * values obtained from the modem.  The enumerations are the same
  * as the values for the fields found in 3GPP TS 27.007
@@ -192,10 +177,6 @@ typedef void (*ofono_sca_query_cb_t)(const struct ofono_error *error,
 					const struct ofono_phone_number *ph,
 					void *data);
 
-typedef void (*ofono_phonebook_export_entries_t)(
-	    const struct ofono_error *error, int num_entries,
-	    const struct ofono_phonebook_entry *entries, void *data);
-
 struct ofono_modem_attribute_ops {
 	void (*query_manufacturer)(struct ofono_modem *modem,
 			ofono_modem_attribute_query_cb_t cb, void *data);
@@ -417,12 +398,22 @@ void ofono_sms_deliver_notify(struct ofono_modem *modem, unsigned char *pdu,
 void ofono_sms_status_notify(struct ofono_modem *modem, unsigned char *pdu,
 				int len, int tpdu_len);
 
+/* Export entries reports results through ofono_phonebook_entry, if an error
+ * occurs, ofono_phonebook_entry should not be called
+ */
 struct ofono_phonebook_ops {
 	void (*export_entries)(struct ofono_modem *modem, const char *storage,
-			ofono_phonebook_export_entries_t cb, void *data);
+				ofono_generic_cb_t cb, void *data);
 };
 
 int ofono_phonebook_register(struct ofono_modem *modem,
 			     struct ofono_phonebook_ops *ops);
 void ofono_phonebook_unregister(struct ofono_modem *modem);
 
+void ofono_phonebook_entry(struct ofono_modem *modem, int index,
+				const char *number, int type,
+				const char *text, int hidden,
+				const char *group,
+				const char *adnumber, int adtype,
+				const char *secondtext, const char *email,
+				const char *sip_uri, const char *tel_uri);
diff --git a/src/phonebook.c b/src/phonebook.c
index 2954711a..6c49df65 100644
--- a/src/phonebook.c
+++ b/src/phonebook.c
@@ -55,7 +55,7 @@ struct phonebook_data {
 };
 
 static const char *storage_support[] = { "\"SM\"", "\"ME\"", NULL };
-static void export_phonebook(DBusMessage *msg, void *data);
+static void export_phonebook(struct ofono_modem *modem);
 
 static struct phonebook_data *phonebook_create()
 {
@@ -108,7 +108,7 @@ static void vcard_printf(GString *str, const char *fmt, ...)
 /* According to RFC 2426, we need escape following characters:
  *  '\n', '\r', ';', ',', '\'.
  */
-static void add_slash(char *dest, char *src, int len_max, int len)
+static void add_slash(char *dest, const char *src, int len_max, int len)
 {
 	int i, j;
 
@@ -136,7 +136,7 @@ static void add_slash(char *dest, char *src, int len_max, int len)
 }
 
 static void vcard_printf_number(GString *entries_vcard_pointer, int type,
-				char *number, int prefer)
+				const char *number, int prefer)
 {
 	char *pref = "", *intl = "";
 	char buf[128];
@@ -151,46 +151,6 @@ static void vcard_printf_number(GString *entries_vcard_pointer, int type,
 	vcard_printf(entries_vcard_pointer, buf, number);
 }
 
-
-static void entry_to_vcard(GString *entries_vcard_pointer,
-				const struct ofono_phonebook_entry *entry)
-{
-	char field[LEN_MAX];
-
-	vcard_printf(entries_vcard_pointer, "BEGIN:VCARD");
-	vcard_printf(entries_vcard_pointer, "VERSION:3.0");
-
-	add_slash(field, entry->text, LEN_MAX, strlen(entry->text));
-
-	vcard_printf(entries_vcard_pointer, "FN:%s", field);
-	vcard_printf_number(entries_vcard_pointer, entry->type,
-			    entry->number, 1);
-
-	if (entry->group) {
-		add_slash(field, entry->group, LEN_MAX, strlen(entry->group));
-		vcard_printf(entries_vcard_pointer, "CATEGORIES:%s", field);
-	}
-
-	if (entry->adtype && entry->adnumber)
-		vcard_printf_number(entries_vcard_pointer, entry->adtype,
-				    entry->adnumber, 0);
-
-	if (entry->email) {
-		add_slash(field, entry->email, LEN_MAX, strlen(entry->email));
-		vcard_printf(entries_vcard_pointer,
-			     "EMAIL;TYPE=INTERNET:%s", field);
-	}
-
-	if (entry->sip_uri) {
-		add_slash(field, entry->sip_uri, LEN_MAX,
-					strlen(entry->sip_uri));
-		vcard_printf(entries_vcard_pointer, "IMPP;TYPE=SIP:%s", field);
-	}
-
-	vcard_printf(entries_vcard_pointer, "END:VCARD");
-	vcard_printf(entries_vcard_pointer, "");
-}
-
 static DBusMessage *generate_export_entries_reply(struct ofono_modem *modem,
 							DBusMessage *msg)
 {
@@ -209,10 +169,50 @@ static DBusMessage *generate_export_entries_reply(struct ofono_modem *modem,
 	return reply;
 }
 
-static void export_phonebook_cb(const struct ofono_error *error,
-				int num_entries,
-				const struct ofono_phonebook_entry *entries,
-				void *data)
+void ofono_phonebook_entry(struct ofono_modem *modem, int index,
+				const char *number, int type,
+				const char *text, int hidden,
+				const char *group,
+				const char *adnumber, int adtype,
+				const char *secondtext, const char *email,
+				const char *sip_uri, const char *tel_uri)
+{
+	struct phonebook_data *phonebook = modem->phonebook;
+	char field[LEN_MAX];
+	int len;
+
+	vcard_printf(phonebook->vcards, "BEGIN:VCARD");
+	vcard_printf(phonebook->vcards, "VERSION:3.0");
+
+	add_slash(field, text, LEN_MAX, strlen(text));
+
+	vcard_printf(phonebook->vcards, "FN:%s", field);
+	vcard_printf_number(phonebook->vcards, type, number, 1);
+
+	if (group && (len = strlen(group))) {
+		add_slash(field, group, LEN_MAX, len);
+		vcard_printf(phonebook->vcards, "CATEGORIES:%s", field);
+	}
+
+	if (adnumber && strlen(adnumber) && adtype != -1)
+		vcard_printf_number(phonebook->vcards, adtype, adnumber, 0);
+
+	if (email && (len = strlen(email))) {
+		add_slash(field, email, LEN_MAX, len);
+		vcard_printf(phonebook->vcards,
+				"EMAIL;TYPE=INTERNET:%s", field);
+	}
+
+	if (sip_uri && (len = strlen(sip_uri))) {
+		add_slash(field, sip_uri, LEN_MAX, len);
+		vcard_printf(phonebook->vcards, "IMPP;TYPE=SIP:%s", field);
+	}
+
+	vcard_printf(phonebook->vcards, "END:VCARD");
+	vcard_printf(phonebook->vcards, "");
+}
+
+static void export_phonebook_cb(const struct ofono_error *error, void *data)
 {
 	struct ofono_modem *modem = data;
 	struct phonebook_data *phonebook = modem->phonebook;
@@ -220,21 +220,15 @@ static void export_phonebook_cb(const struct ofono_error *error,
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		ofono_error("export_entries_one_storage_cb with %s failed",
-			    storage_support[phonebook->storage_index]);
-	else {
-		int num;
-		for (num = 0; num < num_entries; num++)
-			entry_to_vcard(phonebook->vcards, &entries[num]);
-	}
+				storage_support[phonebook->storage_index]);
 
 	phonebook->storage_index++;
-	export_phonebook(phonebook->pending, modem);
+	export_phonebook(modem);
 	return;
 }
 
-static void export_phonebook(DBusMessage *msg, void *data)
+static void export_phonebook(struct ofono_modem *modem)
 {
-	struct ofono_modem *modem = data;
 	struct phonebook_data *phonebook = modem->phonebook;
 	DBusMessage *reply;
 	const char *pb = storage_support[phonebook->storage_index];
@@ -279,7 +273,7 @@ static DBusMessage *export_entries(DBusConnection *conn, DBusMessage *msg,
 	phonebook->storage_index = 0;
 
 	phonebook->pending = dbus_message_ref(msg);
-	export_phonebook(msg, modem);
+	export_phonebook(modem);
 
 	return NULL;
 }
@@ -313,9 +307,9 @@ int ofono_phonebook_register(struct ofono_modem *modem,
 	modem->phonebook->ops = ops;
 
 	if (!g_dbus_register_interface(conn, modem->path,
-				       PHONEBOOK_INTERFACE,
-				       phonebook_methods, phonebook_signals,
-				       NULL, modem, phonebook_destroy)) {
+					PHONEBOOK_INTERFACE,
+					phonebook_methods, phonebook_signals,
+					NULL, modem, phonebook_destroy)) {
 		ofono_error("Could not register Phonebook %s", modem->path);
 
 		phonebook_destroy(modem->phonebook);