From 5fa4cd895c8aa1dc604e8751978f1f6da71d46c4 Mon Sep 17 00:00:00 2001 From: Denis Kenzior Date: Tue, 16 Jun 2009 16:33:05 -0500 Subject: [PATCH] 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 --- drivers/atmodem/phonebook.c | 182 ++++++++++++++---------------------- src/driver.h | 31 +++--- src/phonebook.c | 114 +++++++++++----------- 3 files changed, 135 insertions(+), 192 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_cpbr_notify(GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + GAtResultIter iter; + g_at_result_iter_init(&iter, result); + + 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; + + if (!g_at_result_iter_next_string(&iter, &number)) + continue; + + if (!g_at_result_iter_next_number(&iter, &type)) + continue; + + if (!g_at_result_iter_next_string(&iter, &text)) + continue; + + 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); + } +} + static void at_read_entries_cb(gboolean ok, GAtResult *result, gpointer user_data) { struct cb_data *cbd = user_data; - ofono_phonebook_export_entries_t cb = cbd->cb; + ofono_generic_cb_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; - - 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); - continue; - } - entries[num_entries_real].type = type; - - if (!g_at_result_iter_next_string(&iter, &text)) { - g_free(entries[num_entries_real].number); - 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); - - if (!g_at_result_iter_next_string(&iter, &tel_uri)) - tel_uri = NULL; - entries[num_entries_real].tel_uri = g_strdup(tel_uri); - - 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; + 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);