From be55ba4ab3e706caa9cbc6c854e78d045c723f9a Mon Sep 17 00:00:00 2001 From: Denis Kenzior Date: Fri, 2 Oct 2009 14:02:28 -0500 Subject: [PATCH] Refactor: Move elementary file type checking Every single EF read callback checks the file type reported out of the SIM is what it expects. Instead this should be done in one place and the errors reported accordingly --- include/sim.h | 5 ++- src/cbs.c | 30 ++++++----------- src/message-waiting.c | 45 ++++++++++++-------------- src/network.c | 37 +++++++-------------- src/sim.c | 75 ++++++++++++++++++++----------------------- src/voicecall.c | 9 +++--- 6 files changed, 82 insertions(+), 119 deletions(-) diff --git a/include/sim.h b/include/sim.h index fa5276b6..f76f9d12 100644 --- a/include/sim.h +++ b/include/sim.h @@ -76,9 +76,7 @@ typedef void (*ofono_sim_imsi_cb_t)(const struct ofono_error *error, typedef void (*ofono_sim_ready_notify_cb_t)(void *data); -typedef void (*ofono_sim_file_read_cb_t)(int ok, - enum ofono_sim_file_structure structure, - int total_length, int record, +typedef void (*ofono_sim_file_read_cb_t)(int ok, int total_length, int record, const unsigned char *data, int record_length, void *userdata); @@ -171,6 +169,7 @@ void ofono_sim_set_ready(struct ofono_sim *sim); * Returns 0 if the request could be queued, -1 otherwise. */ int ofono_sim_read(struct ofono_sim *sim, int id, + enum ofono_sim_file_structure expected, ofono_sim_file_read_cb_t cb, void *data); int ofono_sim_write(struct ofono_sim *sim, int id, diff --git a/src/cbs.c b/src/cbs.c index fd050302..9cffb6bf 100644 --- a/src/cbs.c +++ b/src/cbs.c @@ -514,9 +514,7 @@ struct ofono_cbs *ofono_cbs_create(struct ofono_modem *modem, return cbs; } -static void sim_cbmi_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_cbmi_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -528,9 +526,6 @@ static void sim_cbmi_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - if ((length % 2) == 1 || length < 2) return; @@ -570,9 +565,7 @@ static void sim_cbmi_read_cb(int ok, cbs->efcbmi_contents = NULL; } -static void sim_cbmir_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_cbmir_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -585,9 +578,6 @@ static void sim_cbmir_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - if ((length % 4) != 0) return; @@ -629,9 +619,7 @@ static void sim_cbmir_read_cb(int ok, cbs->efcbmir_contents = NULL; } -static void sim_cbmid_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_cbmid_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -643,9 +631,6 @@ static void sim_cbmid_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - if ((length % 2) == 1 || length < 2) return; @@ -689,11 +674,14 @@ static void cbs_got_imsi(struct ofono_cbs *cbs) ofono_debug("Got IMSI: %s", imsi); ofono_sim_read(cbs->sim, SIM_EFCBMI_FILEID, - sim_cbmi_read_cb, cbs); + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_cbmi_read_cb, cbs); ofono_sim_read(cbs->sim, SIM_EFCBMIR_FILEID, - sim_cbmir_read_cb, cbs); + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_cbmir_read_cb, cbs); ofono_sim_read(cbs->sim, SIM_EFCBMID_FILEID, - sim_cbmid_read_cb, cbs); + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_cbmid_read_cb, cbs); } static gboolean reset_base_station_name(gpointer user) diff --git a/src/message-waiting.c b/src/message-waiting.c index c69854ff..ff485899 100644 --- a/src/message-waiting.c +++ b/src/message-waiting.c @@ -295,10 +295,9 @@ static GDBusSignalTable message_waiting_signals[] = { { } }; -static void mw_mwis_read_cb(int ok, - enum ofono_sim_file_structure structure, int total_length, - int record, const unsigned char *data, int record_length, - void *userdata) +static void mw_mwis_read_cb(int ok, int total_length, int record, + const unsigned char *data, + int record_length, void *userdata) { struct ofono_message_waiting *mw = userdata; int i, status; @@ -308,9 +307,7 @@ static void mw_mwis_read_cb(int ok, DBusConnection *conn = ofono_dbus_get_connection(); const char *path = __ofono_atom_get_path(mw->atom); - if (!ok || - structure != OFONO_SIM_FILE_STRUCTURE_FIXED || - record_length < 5) { + if (!ok || record_length < 5) { ofono_error("Unable to read waiting messages numbers " "from SIM"); @@ -356,19 +353,16 @@ static void mw_mwis_read_cb(int ok, mw->efmwis_length = record_length; } -static void mw_mbdn_read_cb(int ok, - enum ofono_sim_file_structure structure, int total_length, - int record, const unsigned char *data, int record_length, - void *userdata) +static void mw_mbdn_read_cb(int ok, int total_length, int record, + const unsigned char *data, + int record_length, void *userdata) { struct ofono_message_waiting *mw = userdata; int i; DBusConnection *conn = ofono_dbus_get_connection(); const char *value; - if (!ok || - structure != OFONO_SIM_FILE_STRUCTURE_FIXED || - record_length < 14 || total_length < record_length) { + if (!ok || record_length < 14 || total_length < record_length) { ofono_error("Unable to read mailbox dialling numbers " "from SIM"); @@ -402,17 +396,14 @@ static void mw_mbdn_read_cb(int ok, mw->efmbdn_length = record_length; } -static void mw_mbi_read_cb(int ok, - enum ofono_sim_file_structure structure, int total_length, - int record, const unsigned char *data, int record_length, - void *userdata) +static void mw_mbi_read_cb(int ok, int total_length, int record, + const unsigned char *data, + int record_length, void *userdata) { struct ofono_message_waiting *mw = userdata; int i, err; - if (!ok || - structure != OFONO_SIM_FILE_STRUCTURE_FIXED || - record_length < 4) { + if (!ok || record_length < 4) { ofono_error("Unable to read mailbox identifies " "from SIM"); @@ -428,7 +419,9 @@ static void mw_mbi_read_cb(int ok, for (i = 0; i < 5 && i < record_length; i++) mw->efmbdn_record_id[i] = data[i]; - err = ofono_sim_read(mw->sim, SIM_EFMBDN_FILEID, mw_mbdn_read_cb, mw); + err = ofono_sim_read(mw->sim, SIM_EFMBDN_FILEID, + OFONO_SIM_FILE_STRUCTURE_FIXED, + mw_mbdn_read_cb, mw); if (err != 0) ofono_error("Unable to read EF-MBDN from SIM"); @@ -740,8 +733,12 @@ void ofono_message_waiting_register(struct ofono_message_waiting *mw) mw->sim = __ofono_atom_get_data(sim_atom); /* Loads MWI states and MBDN from SIM */ - ofono_sim_read(mw->sim, SIM_EFMWIS_FILEID, mw_mwis_read_cb, mw); - ofono_sim_read(mw->sim, SIM_EFMBI_FILEID, mw_mbi_read_cb, mw); + ofono_sim_read(mw->sim, SIM_EFMWIS_FILEID, + OFONO_SIM_FILE_STRUCTURE_FIXED, + mw_mwis_read_cb, mw); + ofono_sim_read(mw->sim, SIM_EFMBI_FILEID, + OFONO_SIM_FILE_STRUCTURE_FIXED, + mw_mbi_read_cb, mw); } __ofono_atom_register(mw->atom, message_waiting_unregister); diff --git a/src/network.c b/src/network.c index 9f2e3ff5..0e5d55b4 100644 --- a/src/network.c +++ b/src/network.c @@ -1216,9 +1216,7 @@ static void signal_strength_callback(const struct ofono_error *error, ofono_netreg_strength_notify(netreg, strength); } -static void sim_opl_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_opl_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -1233,9 +1231,6 @@ static void sim_opl_read_cb(int ok, return; } - if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED) - return; - if (record_length < 8 || length < record_length) return; @@ -1260,9 +1255,7 @@ optimize: } } -static void sim_pnn_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_pnn_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -1272,9 +1265,6 @@ static void sim_pnn_read_cb(int ok, if (!ok) goto check; - if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED) - return; - if (length < 3 || record_length < 3 || length < record_length) return; @@ -1295,12 +1285,11 @@ check: * is present. */ if (netreg->eons && !sim_eons_pnn_is_empty(netreg->eons)) ofono_sim_read(netreg->sim, SIM_EFOPL_FILEID, - sim_opl_read_cb, netreg); + OFONO_SIM_FILE_STRUCTURE_FIXED, + sim_opl_read_cb, netreg); } -static void sim_spdi_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_spdi_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -1310,9 +1299,6 @@ static void sim_spdi_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - netreg->spdi = sim_spdi_new(data, length); if (!current) @@ -1336,9 +1322,7 @@ static void sim_spdi_read_cb(int ok, } } -static void sim_spn_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_spn_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -1349,9 +1333,6 @@ static void sim_spn_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - dcbyte = data[0]; /* TS 31.102 says: @@ -1382,7 +1363,9 @@ static void sim_spn_read_cb(int ok, } netreg->spname = spn; - ofono_sim_read(netreg->sim, SIM_EFSPDI_FILEID, sim_spdi_read_cb, netreg); + ofono_sim_read(netreg->sim, SIM_EFSPDI_FILEID, + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_spdi_read_cb, netreg); if (dcbyte & SIM_EFSPN_DC_HOME_PLMN_BIT) netreg->flags |= NETWORK_REGISTRATION_FLAG_HOME_SHOW_PLMN; @@ -1601,8 +1584,10 @@ void ofono_netreg_register(struct ofono_netreg *netreg) netreg->sim = __ofono_atom_get_data(sim_atom); ofono_sim_read(netreg->sim, SIM_EFPNN_FILEID, + OFONO_SIM_FILE_STRUCTURE_FIXED, sim_pnn_read_cb, netreg); ofono_sim_read(netreg->sim, SIM_EFSPN_FILEID, + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, sim_spn_read_cb, netreg); } diff --git a/src/sim.c b/src/sim.c index 62b88811..b2fbe262 100644 --- a/src/sim.c +++ b/src/sim.c @@ -762,9 +762,7 @@ static gboolean numbers_list_equal(GSList *a, GSList *b) return TRUE; } -static void sim_msisdn_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_msisdn_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -775,9 +773,6 @@ static void sim_msisdn_read_cb(int ok, if (!ok) goto check; - if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED) - return; - if (record_length < 14 || length < record_length) return; @@ -827,9 +822,7 @@ check: sim->new_numbers = NULL; } -static void sim_ad_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_ad_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -841,9 +834,6 @@ static void sim_ad_read_cb(int ok, if (!ok) return; - if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) - return; - if (length < 4) return; @@ -868,9 +858,7 @@ static gint service_number_compare(gconstpointer a, gconstpointer b) return strcmp(sdn->id, id); } -static void sim_sdn_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_sdn_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -885,9 +873,6 @@ static void sim_sdn_read_cb(int ok, if (!ok) goto check; - if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED) - return; - if (record_length < 14 || length < record_length) return; @@ -947,7 +932,7 @@ check: static void sim_own_numbers_update(struct ofono_sim *sim) { - ofono_sim_read(sim, SIM_EFMSISDN_FILEID, + ofono_sim_read(sim, SIM_EFMSISDN_FILEID, OFONO_SIM_FILE_STRUCTURE_FIXED, sim_msisdn_read_cb, sim); } @@ -957,8 +942,11 @@ static void sim_ready(void *user) sim_own_numbers_update(sim); - ofono_sim_read(sim, SIM_EFAD_FILEID, sim_ad_read_cb, sim); - ofono_sim_read(sim, SIM_EFSDN_FILEID, sim_sdn_read_cb, sim); + ofono_sim_read(sim, SIM_EFAD_FILEID, + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_ad_read_cb, sim); + ofono_sim_read(sim, SIM_EFSDN_FILEID, OFONO_SIM_FILE_STRUCTURE_FIXED, + sim_sdn_read_cb, sim); } static void sim_imsi_cb(const struct ofono_error *error, const char *imsi, @@ -1030,15 +1018,13 @@ static void sim_pin_check(struct ofono_sim *sim) sim->driver->query_passwd_state(sim, sim_pin_query_cb, sim); } -static void sim_efli_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_efli_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { struct ofono_sim *sim = userdata; - if (!ok || structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT) + if (!ok) return; sim->efli = g_memdup(data, length); @@ -1143,9 +1129,7 @@ static char **concat_lang_prefs(GSList *a, GSList *b) return ret; } -static void sim_efpl_read_cb(int ok, - enum ofono_sim_file_structure structure, - int length, int record, +static void sim_efpl_read_cb(int ok, int length, int record, const unsigned char *data, int record_length, void *userdata) { @@ -1156,8 +1140,7 @@ static void sim_efpl_read_cb(int ok, GSList *efli = NULL; GSList *efpl = NULL; - if (!ok || structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT || - length < 2) + if (!ok || length < 2) goto skip_efpl; efpl = parse_language_list(data, length); @@ -1226,8 +1209,12 @@ static void sim_retrieve_efli_and_efpl(struct ofono_sim *sim) * However we don't depend on the user interface and so * need to read both files now. */ - ofono_sim_read(sim, SIM_EFLI_FILEID, sim_efli_read_cb, sim); - ofono_sim_read(sim, SIM_EFPL_FILEID, sim_efpl_read_cb, sim); + ofono_sim_read(sim, SIM_EFLI_FILEID, + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_efli_read_cb, sim); + ofono_sim_read(sim, SIM_EFPL_FILEID, + OFONO_SIM_FILE_STRUCTURE_TRANSPARENT, + sim_efpl_read_cb, sim); } static void sim_op_error(struct ofono_sim *sim) @@ -1239,7 +1226,7 @@ static void sim_op_error(struct ofono_sim *sim) if (op->is_read == TRUE) ((ofono_sim_file_read_cb_t) op->cb) - (0, 0, 0, 0, 0, 0, op->userdata); + (0, 0, 0, 0, 0, op->userdata); else ((ofono_sim_file_write_cb_t) op->cb) (0, op->userdata); @@ -1286,8 +1273,7 @@ static void sim_op_retrieve_cb(const struct ofono_error *error, return; } - cb(1, op->structure, op->length, op->current, - data, op->record_length, op->userdata); + cb(1, op->length, op->current, data, op->record_length, op->userdata); if (op->cache && imsi) { char *path = g_strdup_printf(SIM_CACHE_PATH, imsi, op->id); @@ -1371,6 +1357,13 @@ static void sim_op_info_cb(const struct ofono_error *error, int length, return; } + if (structure != op->structure) { + ofono_error("Requested file structure differs from SIM: %x", + op->id); + sim_op_error(sim); + return; + } + /* TS 11.11, Section 9.3 */ update = file_access_condition_decode(access[0] & 0xf); rehabilitate = file_access_condition_decode((access[2] >> 4) & 0xf); @@ -1478,9 +1471,10 @@ static gboolean sim_op_check_cached(struct ofono_sim *sim) if (record_length == 0 || file_length < record_length) goto cleanup; - if (error_type != OFONO_ERROR_TYPE_NO_ERROR) { + if (error_type != OFONO_ERROR_TYPE_NO_ERROR || + structure != op->structure) { ret = TRUE; - cb(0, 0, 0, 0, 0, 0, 0); + cb(0, 0, 0, 0, 0, 0); goto cleanup; } @@ -1492,9 +1486,8 @@ static gboolean sim_op_check_cached(struct ofono_sim *sim) goto cleanup; for (record = 0; record < file_length / record_length; record++) { - cb(1, structure, file_length, record + 1, - &buffer[record * record_length], record_length, - op->userdata); + cb(1, file_length, record + 1, &buffer[record * record_length], + record_length, op->userdata); } ret = TRUE; @@ -1563,6 +1556,7 @@ static gboolean sim_op_next(gpointer user_data) } int ofono_sim_read(struct ofono_sim *sim, int id, + enum ofono_sim_file_structure expected_type, ofono_sim_file_read_cb_t cb, void *data) { struct sim_file_op *op; @@ -1589,6 +1583,7 @@ int ofono_sim_read(struct ofono_sim *sim, int id, op = g_new0(struct sim_file_op, 1); op->id = id; + op->structure = expected_type; op->cb = cb; op->userdata = data; op->is_read = TRUE; diff --git a/src/voicecall.c b/src/voicecall.c index bc6986c5..eff6321b 100644 --- a/src/voicecall.c +++ b/src/voicecall.c @@ -1705,8 +1705,7 @@ static void set_new_ecc(struct ofono_voicecall *vc) emit_en_list_changed(vc); } -static void ecc_read_cb(int ok, enum ofono_sim_file_structure structure, - int total_length, int record, const unsigned char *data, +static void ecc_read_cb(int ok, int total_length, int record, const unsigned char *data, int record_length, void *userdata) { struct ofono_voicecall *vc = userdata; @@ -1718,8 +1717,7 @@ static void ecc_read_cb(int ok, enum ofono_sim_file_structure structure, if (!ok) goto check; - if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED || - record_length < 4 || total_length < record_length) { + if (record_length < 4 || total_length < record_length) { ofono_error("Unable to read emergency numbers from SIM"); return; } @@ -1863,7 +1861,8 @@ static void sim_watch(struct ofono_atom *atom, return; } - ofono_sim_read(sim, SIM_EFECC_FILEID, ecc_read_cb, vc); + ofono_sim_read(sim, SIM_EFECC_FILEID, OFONO_SIM_FILE_STRUCTURE_FIXED, + ecc_read_cb, vc); } void ofono_voicecall_register(struct ofono_voicecall *vc)