[PATCH] Read EF-PNN, EF-OPL and override network names accordingly.

[PATCH] Read EF-PNN, EF-OPL and override network names accordingly.

[Andrzej Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 16480 bytes --]

---
 src/driver.h  |    2 +
 src/network.c |   54 ++++++----
 src/sim.c     |  315 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 src/sim.h     |    4 +
 4 files changed, 345 insertions(+), 30 deletions(-)

diff --git a/src/driver.h b/src/driver.h
index f324c1c..595c980 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -84,6 +84,8 @@ struct ofono_network_operator {
 	char mnc[OFONO_MAX_MNC_LENGTH + 1];
 	int status;
 	int tech;
+
+	const char *override_name;
 };
 
 /* 27.007 Section 7.11 Call Forwarding */
diff --git a/src/network.c b/src/network.c
index 5419a97..eab3cde 100644
--- a/src/network.c
+++ b/src/network.c
@@ -339,13 +339,15 @@ static char *get_operator_display_name(struct ofono_modem *modem)
 		return name;
 	}
 
+	plmn = netreg->current_operator->name;
+	if (netreg->current_operator->override_name)
+		plmn = netreg->current_operator->override_name;
+
 	if (!netreg->spname || strlen(netreg->spname) == 0) {
-		g_strlcpy(name, netreg->current_operator->name, len);
+		g_strlcpy(name, plmn, len);
 		return name;
 	}
 
-	plmn = netreg->current_operator->name;
-
 	home_or_spdi =
 		(netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED) ||
 		ofono_operator_in_spdi(modem, netreg->current_operator);
@@ -383,20 +385,22 @@ static void set_network_operator_name(struct ofono_modem *modem,
 	strncpy(op->name, name, OFONO_MAX_OPERATOR_NAME_LENGTH);
 	op->name[OFONO_MAX_OPERATOR_NAME_LENGTH] = '\0';
 
-	path = network_operator_build_path(modem, op);
+	if (!op->override_name) {
+		path = network_operator_build_path(modem, op);
 
-	dbus_gsm_signal_property_changed(conn, path,
-				NETWORK_OPERATOR_INTERFACE,
-				"Name", DBUS_TYPE_STRING,
-				&name);
+		dbus_gsm_signal_property_changed(conn, path,
+					NETWORK_OPERATOR_INTERFACE,
+					"Name", DBUS_TYPE_STRING,
+					&name);
 
-	if (op == netreg->current_operator) {
-		operator = get_operator_display_name(modem);
+		if (op == netreg->current_operator) {
+			operator = get_operator_display_name(modem);
 
-		dbus_gsm_signal_property_changed(conn, modem->path,
-					NETWORK_REGISTRATION_INTERFACE,
-					"Operator", DBUS_TYPE_STRING,
-					&operator);
+			dbus_gsm_signal_property_changed(conn, modem->path,
+						NETWORK_REGISTRATION_INTERFACE,
+						"Operator", DBUS_TYPE_STRING,
+						&operator);
+		}
 	}
 }
 
@@ -413,6 +417,9 @@ static DBusMessage *network_operator_get_properties(DBusConnection *conn,
 	const char *status =
 		network_operator_status_to_string(op->operator->status);
 
+	if (op->operator->override_name)
+		name = op->operator->override_name;
+
 	reply = dbus_message_new_method_return(msg);
 	if (!reply)
 		return NULL;
@@ -852,6 +859,13 @@ void ofono_network_registration_notify(struct ofono_modem *modem, int status,
 	}
 }
 
+static void network_operator_name_override(struct ofono_modem *modem,
+					struct ofono_network_operator *op)
+{
+	op->override_name =
+		ofono_operator_name_sim_override(modem, op->mcc, op->mnc);
+}
+
 static void operator_list_callback(const struct ofono_error *error, int total,
 					const struct ofono_network_operator *list,
 					void *data)
@@ -890,11 +904,12 @@ static void operator_list_callback(const struct ofono_error *error, int total,
 		} else {
 			/* New operator */
 			struct ofono_network_operator *op =
-				g_try_new0(struct ofono_network_operator, 1);
+				g_memdup(&list[i],
+					sizeof(struct ofono_network_operator));
 			if (!op)
 				continue;
 
-			memcpy(op, &list[i], sizeof(struct ofono_network_operator));
+			network_operator_name_override(modem, op);
 
 			if (network_operator_dbus_register(modem, op)) {
 				n = g_slist_prepend(n, op);
@@ -968,15 +983,14 @@ static void current_operator_callback(const struct ofono_error *error,
 
 	if (current) {
 		netreg->current_operator =
-			g_try_new0(struct ofono_network_operator, 1);
-
+			g_memdup(current,
+				sizeof(struct ofono_network_operator));
 		if (!netreg->current_operator) {
 			ofono_error("Unable to allocate current operator");
 			return;
 		}
 
-		memcpy(netreg->current_operator, current,
-			sizeof(struct ofono_network_operator));
+		network_operator_name_override(modem, netreg->current_operator);
 
 		netreg->operator_list = g_slist_append(netreg->operator_list,
 						netreg->current_operator);
diff --git a/src/sim.c b/src/sim.c
index a5d2164..13557bf 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -58,6 +58,16 @@ struct sim_manager_data {
 	int own_numbers_num;
 	int own_numbers_size;
 	int own_numbers_current;
+
+	GSList *opl;
+	int opl_num;
+	int opl_size;
+	int opl_current;
+
+	struct pnn_operator *pnn;
+	int pnn_num;
+	int pnn_size;
+	int pnn_current;
 };
 
 static char **get_own_numbers(GSList *own_numbers)
@@ -120,6 +130,11 @@ static const guint8 *ber_tlv_find_by_tag(const guint8 *pdu, guint8 in_tag,
 	return NULL;
 }
 
+struct pnn_operator {
+	char *longname;
+	char *shortname;
+};
+
 static struct sim_manager_data *sim_manager_create()
 {
 	return g_try_new0(struct sim_manager_data, 1);
@@ -129,6 +144,7 @@ static void sim_manager_destroy(gpointer userdata)
 {
 	struct ofono_modem *modem = userdata;
 	struct sim_manager_data *data = modem->sim_manager;
+	int i;
 
 	if (data->imsi) {
 		g_free(data->imsi);
@@ -151,6 +167,24 @@ static void sim_manager_destroy(gpointer userdata)
 		g_slist_free(data->spdi);
 		data->spdi = NULL;
 	}
+
+	if (data->opl) {
+		g_slist_foreach(data->opl, (GFunc)g_free, NULL);
+		g_slist_free(data->opl);
+		data->opl = NULL;
+	}
+
+	if (data->pnn) {
+		for (i = 0; i < data->pnn_num; i ++) {
+			if (data->pnn[i].longname)
+				g_free(data->pnn[i].longname);
+			if (data->pnn[i].shortname)
+				g_free(data->pnn[i].shortname);
+		}
+		g_free(data->pnn);
+		data->pnn = NULL;
+		data->pnn_num = 0;
+	}
 }
 
 static DBusMessage *sim_get_properties(DBusConnection *conn,
@@ -199,6 +233,37 @@ static GDBusMethodTable sim_manager_methods[] = {
 
 static GDBusSignalTable sim_manager_signals[] = { { } };
 
+static char *network_name_parse(const unsigned char *buffer, int length)
+{
+	unsigned char *endp;
+	unsigned int dcs;
+
+	if (length < 1)
+		return NULL;
+
+	dcs = *buffer ++;
+	length --;
+
+	if (dcs & (1 << 3)) {
+		/* TODO: "The MS should add the letters for the Country's
+		 * Initials and a separator (e.g. a space)" */
+	}
+
+	switch (dcs & (7 << 4)) {
+	case 0x00:
+		endp = memchr(buffer, 0xff, length);
+		if (endp)
+			length = endp - buffer;
+		return convert_gsm_to_utf8(buffer, length,
+				NULL, NULL, 0xff);
+	case 0x10:
+		return convert_ucs2_to_utf8(buffer, length,
+				NULL, NULL, 0xffff);
+	}
+
+	return NULL;
+}
+
 static char *sim_alpha_field_parse(const unsigned char *buffer, int length)
 {
 	long utf_len, char_len;
@@ -311,6 +376,8 @@ static char *sim_alpha_field_parse(const unsigned char *buffer, int length)
 enum sim_fileids {
 	SIM_EFMSISDN_FILEID = 0x6f40,
 	SIM_EFSPN_FILEID = 0x6f46,
+	SIM_EFPNN_FILEID = 0x6fc5,
+	SIM_EFOPL_FILEID = 0x6fc6,
 	SIM_EFSPDI_FILEID = 0x6fcd,
 };
 
@@ -494,16 +561,15 @@ static gboolean sim_retrieve_own_number(void *user_data)
 	return FALSE;
 }
 
-struct spdi_operator {
+struct sim_operator {
 	char mcc[OFONO_MAX_MCC_LENGTH + 1];
 	char mnc[OFONO_MAX_MNC_LENGTH + 1];
 };
 
-static struct spdi_operator *spdi_operator_alloc(const guint8 *bcd)
+static void parse_mcc_mnc(struct sim_operator *oper, const guint8 *bcd)
 {
-	struct spdi_operator *spdi = g_new0(struct spdi_operator, 1);
-	char *mcc = spdi->mcc;
-	char *mnc = spdi->mnc;
+	char *mcc = oper->mcc;
+	char *mnc = oper->mnc;
 	guint8 digit;
 
 	digit = (bcd[0] >> 0) & 0xf;
@@ -524,14 +590,21 @@ static struct spdi_operator *spdi_operator_alloc(const guint8 *bcd)
 	digit = (bcd[1] >> 4) & 0xf;
 	if (digit != 0xf)
 		*mnc ++ = '0' + digit;
+}
+
+static struct sim_operator *sim_operator_alloc(const guint8 *bcd)
+{
+	struct sim_operator *spdi = g_new0(struct sim_operator, 1);
+
+	parse_mcc_mnc(spdi, bcd);
 
 	return spdi;
 }
 
 static gint spdi_operator_compare(gconstpointer a, gconstpointer b)
 {
-	const struct spdi_operator *opa = a;
-	const struct spdi_operator *opb = b;
+	const struct sim_operator *opa = a;
+	const struct sim_operator *opb = b;
 	gint r;
 
 	if (r = strcmp(opa->mcc, opb->mcc))
@@ -544,7 +617,7 @@ gboolean ofono_operator_in_spdi(struct ofono_modem *modem,
 				const struct ofono_network_operator *op)
 {
 	struct sim_manager_data *sim = modem->sim_manager;
-	struct spdi_operator spdi_op;
+	struct sim_operator spdi_op;
 
 	if (!sim)
 		return FALSE;
@@ -563,7 +636,7 @@ static void sim_spdi_read_cb(const struct ofono_error *error,
 	struct ofono_modem *modem = data;
 	struct sim_manager_data *sim = modem->sim_manager;
 	const guint8 *plmn_list;
-	struct spdi_operator *spdi;
+	struct sim_operator *spdi;
 	GSList *l;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR || length <= 5)
@@ -580,7 +653,7 @@ static void sim_spdi_read_cb(const struct ofono_error *error,
 			continue;
 
 		sim->spdi = g_slist_insert_sorted(sim->spdi,
-				spdi_operator_alloc(plmn_list),
+				sim_operator_alloc(plmn_list),
 				spdi_operator_compare);
 	}
 
@@ -615,6 +688,225 @@ static gboolean sim_retrieve_spdi(void *user_data)
 	return FALSE;
 }
 
+struct opl_operator {
+	struct sim_operator mcc_mnc;
+	guint16 lac_tac_low;
+	guint16 lac_tac_high;
+	guint8 id;
+};
+
+static struct opl_operator *opl_operator_alloc(const guint8 *record)
+{
+	struct opl_operator *oper = g_new0(struct opl_operator, 1);
+
+	parse_mcc_mnc(&oper->mcc_mnc, record);
+	record += 3;
+
+	oper->lac_tac_low = (record[0] << 8) | record[1];
+	record += 2;
+	oper->lac_tac_high = (record[0] << 8) | record[1];
+	record += 2;
+
+	oper->id = record[0];
+	if (!oper->id) {
+		/* TODO: name to be taken from other sources, see TS 22.101 */
+	}
+
+	return oper;
+}
+
+static gint opl_operator_compare(gconstpointer a, gconstpointer b)
+{
+	const struct opl_operator *opa = a;
+	const struct sim_operator *opb = b;
+	int i;
+
+	for (i = 0; opb->mcc[i] | opa->mcc_mnc.mcc[i]; i ++)
+		if (opb->mcc[i] != opa->mcc_mnc.mcc[i] &&
+				!(opa->mcc_mnc.mcc[i] == '0' + 0xd &&
+					opb->mcc[i]))
+			return opa->mcc_mnc.mcc[i] - opb->mcc[i];
+	for (i = 0; opb->mnc[i] | opa->mcc_mnc.mnc[i]; i ++)
+		if (opb->mnc[i] != opa->mcc_mnc.mnc[i] &&
+				!(opa->mcc_mnc.mnc[i] == '0' + 0xd &&
+					opb->mnc[i]))
+			return opa->mcc_mnc.mnc[i] - opb->mnc[i];
+
+	if (opa->lac_tac_low > 0x0000 || opa->lac_tac_high < 0xfffe)
+		return 1;
+
+	return 0;
+}
+
+static void sim_opl_read_cb(const struct ofono_error *error,
+		const unsigned char *sdata, int length, void *data)
+{
+	struct ofono_modem *modem = data;
+	struct sim_manager_data *sim = modem->sim_manager;
+	struct opl_operator *oper;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		goto skip;
+
+	if (length < sim->opl_size)
+		goto skip;
+
+	oper = opl_operator_alloc(sdata);
+	if (oper->id > sim->pnn_num) {
+		g_free(oper);
+		goto skip;
+	}
+
+	sim->opl = g_slist_prepend(sim->opl, oper);
+
+skip:
+	sim->opl_current ++;
+	if (sim->opl_current < sim->opl_num)
+		sim->ops->read_file_linear(modem, SIM_EFOPL_FILEID,
+						sim->opl_current,
+						sim->opl_size,
+						sim_opl_read_cb, modem);
+	else
+		/* All records retrieved */
+		if (sim->opl)
+			sim->opl = g_slist_reverse(sim->opl);
+}
+
+static void sim_opl_info_cb(const struct ofono_error *error, int length,
+				enum ofono_sim_file_structure structure,
+				int record_length, void *data)
+{
+	struct ofono_modem *modem = data;
+	struct sim_manager_data *sim = modem->sim_manager;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR || length < 8 ||
+			record_length < 8 ||
+			structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
+		return;
+
+	sim->opl_current = 0;
+	sim->opl_size = record_length;
+	sim->opl_num = length / record_length;
+	sim->ops->read_file_linear(modem, SIM_EFOPL_FILEID, 0,
+			record_length, sim_opl_read_cb, modem);
+}
+
+static gboolean sim_retrieve_opl(void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct sim_manager_data *sim = modem->sim_manager;
+
+	sim->ops->read_file_info(modem, SIM_EFOPL_FILEID,
+			sim_opl_info_cb, modem);
+
+	return FALSE;
+}
+
+const char *ofono_operator_name_sim_override(struct ofono_modem *modem,
+		const char *mcc, const char *mnc)
+{
+	struct sim_manager_data *sim = modem->sim_manager;
+	struct sim_operator op;
+	GSList *l;
+	const struct opl_operator *opl_op;
+
+	g_strlcpy(op.mcc, mcc, sizeof(op.mcc));
+	g_strlcpy(op.mnc, mnc, sizeof(op.mnc));
+
+	l = g_slist_find_custom(sim->opl, &op, opl_operator_compare);
+	if (!l)
+		return NULL;
+	opl_op = l->data;
+
+	return sim->pnn[opl_op->id - 1].longname;
+}
+
+static gboolean pnn_operator_parse(struct pnn_operator *oper,
+				const guint8 *tlv, int length)
+{
+	const char *name;
+	int namelength;
+
+	name = ber_tlv_find_by_tag(tlv, 0x43, length, &namelength);
+	if (!name || !namelength)
+		return FALSE;
+	oper->longname = network_name_parse(name, namelength);
+
+	name = ber_tlv_find_by_tag(tlv, 0x45, length, &namelength);
+	if (name && namelength)
+		oper->shortname = network_name_parse(name, namelength);
+
+	if (ber_tlv_find_by_tag(tlv, 0x80, length, &namelength))
+		ofono_debug("%i octets of addition PLMN information "
+				"present in EF-PNN");
+
+	return TRUE;
+}
+
+static void sim_pnn_read_cb(const struct ofono_error *error,
+		const unsigned char *pnndata, int length, void *data)
+{
+	struct ofono_modem *modem = data;
+	struct sim_manager_data *sim = modem->sim_manager;
+	struct opl_operator *oper;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		goto skip;
+
+	if (length < sim->pnn_size)
+		goto skip;
+
+	pnn_operator_parse(&sim->pnn[sim->pnn_current], pnndata, length);
+
+skip:
+	sim->pnn_current ++;
+	if (sim->pnn_current < sim->pnn_num)
+		sim->ops->read_file_linear(modem, SIM_EFPNN_FILEID,
+						sim->pnn_current,
+						sim->pnn_size,
+						sim_pnn_read_cb, modem);
+	else
+		/* All records retrieved */
+		/* We now need EF-OPL if it's there for PNN to be
+		 * useful.  */
+		sim_retrieve_opl(modem);
+}
+
+static void sim_pnn_info_cb(const struct ofono_error *error, int length,
+				enum ofono_sim_file_structure structure,
+				int record_length, void *data)
+{
+	struct ofono_modem *modem = data;
+	struct sim_manager_data *sim = modem->sim_manager;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR || length < 3 ||
+			record_length < 3 ||
+			structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
+		/* If PNN is not present then OPL is not useful, don't
+		 * retrieve it.  If OPL is not there then PNN[1] will
+		 * still be used for the HPLMN and/or EHPLMN, if PNN
+		 * is present.  */
+		return;
+
+	sim->pnn_current = 0;
+	sim->pnn_size = record_length;
+	sim->pnn_num = length / record_length;
+	sim->pnn = g_new0(struct pnn_operator, sim->pnn_num);
+	sim->ops->read_file_linear(modem, SIM_EFPNN_FILEID, 0,
+			record_length, sim_pnn_read_cb, modem);
+}
+
+static gboolean sim_retrieve_pnn(void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct sim_manager_data *sim = modem->sim_manager;
+
+	sim->ops->read_file_info(modem, SIM_EFPNN_FILEID,
+			sim_pnn_info_cb, modem);
+
+	return FALSE;
+}
+
 static void initialize_sim_manager(struct ofono_modem *modem)
 {
 	DBusConnection *conn = dbus_gsm_connection();
@@ -647,6 +939,9 @@ static void initialize_sim_manager(struct ofono_modem *modem)
 
 	if (modem->sim_manager->ops->read_file_transparent)
 		g_timeout_add(0, sim_retrieve_spdi, modem);
+
+	if (modem->sim_manager->ops->read_file_linear)
+		g_timeout_add(0, sim_retrieve_pnn, modem);
 }
 
 int ofono_sim_manager_register(struct ofono_modem *modem,
diff --git a/src/sim.h b/src/sim.h
index 251a339..ecb809a 100644
--- a/src/sim.h
+++ b/src/sim.h
@@ -31,3 +31,7 @@ int ofono_spn_update_notify_unregister(struct ofono_modem *modem,
 
 gboolean ofono_operator_in_spdi(struct ofono_modem *modem,
 				const struct ofono_network_operator *op);
+
+const char *ofono_operator_name_sim_override(struct ofono_modem *modem,
+						const char *mcc,
+						const char *mnc);
-- 
1.6.0

Re: [PATCH] Read EF-PNN, EF-OPL and override network names accordingly.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 315 bytes --]

Hi,

Patch has been applied.  However, I've completely (juding by 40 or so commits) 
refactored how SIM filesystem is handled in oFono so I might have introduced 
some bugs.  Could you test to make sure it is working as intended?

Can you submit some unit tests with sample PNN/OPL data?

Regards,
-Denis

Re: [PATCH] Read EF-PNN, EF-OPL and override network names accordingly.

[Andrzej Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

2009/7/14 Denis Kenzior <denkenz@gmail.com>:
> Patch has been applied.  However, I've completely (juding by 40 or so commits)
> refactored how SIM filesystem is handled in oFono so I might have introduced
> some bugs.  Could you test to make sure it is working as intended?

Attaching some fixes for the new code, the test and a clean-up patch
(feel free to drop parts).  Note all the recent commits broke
bisectability badly.

Wouldn't it be useful for the service provider name to be available as
SimManager property?

Regards

[-- Attachment #2: 0001-Make-PNN-OPL-work.patch --]
[-- Type: application/octet-stream, Size: 1643 bytes --]

From 2ccd0f29394d3e66bd553e19cf739a8ce5a8d700 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 18 Jul 2009 01:42:31 +0200
Subject: [PATCH] Make PNN / OPL work.

---
 src/network.c |    4 ++--
 src/simutil.c |   12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/network.c b/src/network.c
index 56ca924..969c637 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1206,12 +1206,12 @@ static void sim_opl_read_cb(struct ofono_modem *modem, int ok,
 	if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
 		return;
 
-	if (length < 8 || record_length < 8 || length < record_length)
+	if (record_length < 8 || length < record_length)
 		return;
 
 	total = length / record_length;
 
-	sim_eons_add_pnn_record(netreg->eons, record, data, record_length);
+	sim_eons_add_opl_record(netreg->eons, data, record_length);
 
 	if (record != total)
 		return;
diff --git a/src/simutil.c b/src/simutil.c
index 16fad16..26d1bad 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -338,13 +338,17 @@ static struct sim_eons_operator_info *
 
 		for (i = 0; i < OFONO_MAX_MCC_LENGTH; i++)
 			if (mcc[i] != opl->mcc[i] &&
-					!(opl->mcc[i] == 'd' && mcc[i]))
-				continue;
+					!(opl->mcc[i] == 'b' && mcc[i]))
+				break;
+		if (i < OFONO_MAX_MCC_LENGTH)
+			continue;
 
 		for (i = 0; i < OFONO_MAX_MNC_LENGTH; i++)
 			if (mnc[i] != opl->mnc[i] &&
-					!(opl->mnc[i] == 'd' && mnc[i]))
-				continue;
+					!(opl->mnc[i] == 'b' && mnc[i]))
+				break;
+		if (i < OFONO_MAX_MNC_LENGTH)
+			continue;
 
 		if (opl->lac_tac_low == 0 && opl->lac_tac_high == 0xfffe)
 			break;
-- 
1.6.0


[-- Attachment #3: 0002-Clean-up-in-sim.c.patch --]
[-- Type: application/octet-stream, Size: 6663 bytes --]

From 4f14a94ffce861e5a0c412acf9440c6b9a6ff725 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 18 Jul 2009 01:42:54 +0200
Subject: [PATCH] Clean-up in sim.c.

---
 src/sim.c |   26 +++++---------------------
 1 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index bd92836..86f2e98 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -65,13 +65,6 @@ struct sim_manager_data {
 	GSList *ready_notify;
 	gboolean ready;
 	GQueue *simop_q;
-
-	GSList *spdi;
-
-	GSList *opl;
-	int opl_num;
-	int opl_size;
-	int opl_current;
 };
 
 static char **get_own_numbers(GSList *own_numbers)
@@ -127,18 +120,6 @@ static void sim_manager_destroy(gpointer userdata)
 		g_queue_free(data->simop_q);
 		data->simop_q = NULL;
 	}
-
-	if (data->spdi) {
-		g_slist_foreach(data->spdi, (GFunc)g_free, NULL);
-		g_slist_free(data->spdi);
-		data->spdi = NULL;
-	}
-
-	if (data->opl) {
-		g_slist_foreach(data->opl, (GFunc)g_free, NULL);
-		g_slist_free(data->opl);
-		data->opl = NULL;
-	}
 }
 
 static DBusMessage *sim_get_properties(DBusConnection *conn,
@@ -181,7 +162,10 @@ static GDBusMethodTable sim_manager_methods[] = {
 	{ }
 };
 
-static GDBusSignalTable sim_manager_signals[] = { { } };
+static GDBusSignalTable sim_manager_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
 
 
 static void sim_msisdn_read_cb(struct ofono_modem *modem, int ok,
@@ -202,7 +186,7 @@ static void sim_msisdn_read_cb(struct ofono_modem *modem, int ok,
 	if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
 		return;
 
-	if (length < 14 || record_length < 14 || length < record_length)
+	if (record_length < 14 || length < record_length)
 		return;
 
 	total = length / record_length;
diff --git a/src/network.c b/src/network.c
index 1152b28..e264374 100644
--- a/src/network.c
+++ b/src/network.c
@@ -89,7 +89,7 @@ static void registration_status_callback(const struct ofono_error *error,
 struct network_operator_data {
 	struct ofono_network_operator *info;
 	struct ofono_modem *modem;
-	struct sim_eons_operator_info *eons_info;
+	const struct sim_eons_operator_info *eons_info;
 };
 
 static inline const char *network_operator_status_to_string(int status)
@@ -427,11 +427,11 @@ static void set_network_operator_name(struct ofono_modem *modem,
 
 static void set_network_operator_eons_info(struct ofono_modem *modem,
 				struct network_operator_data *opd,
-				struct sim_eons_operator_info *eons_info)
+				const struct sim_eons_operator_info *eons_info)
 {
 	struct network_registration_data *netreg = modem->network_registration;
 	DBusConnection *conn = dbus_gsm_connection();
-	struct sim_eons_operator_info *old_eons_info = opd->eons_info;
+	const struct sim_eons_operator_info *old_eons_info = opd->eons_info;
 	const char *path;
 	const char *oldname;
 	const char *newname;
@@ -1226,7 +1226,7 @@ static void sim_opl_read_cb(struct ofono_modem *modem, int ok,
 
 	for (l = netreg->operator_list; l; l = l->next) {
 		struct network_operator_data *opd = l->data;
-		struct sim_eons_operator_info *eons_info;
+		const struct sim_eons_operator_info *eons_info;
 
 		eons_info = sim_eons_lookup(netreg->eons, opd->info->mcc,
 						opd->info->mnc);
diff --git a/src/simutil.c b/src/simutil.c
index 26d1bad..1430ec3 100644
--- a/src/simutil.c
+++ b/src/simutil.c
@@ -32,6 +32,13 @@
 #include "util.h"
 #include "smsutil.h"
 
+struct sim_eons {
+	struct sim_eons_operator_info *pnn_list;
+	GSList *opl_list;
+	gboolean pnn_valid;
+	int pnn_max;
+};
+
 struct spdi_operator {
 	char mcc[OFONO_MAX_MCC_LENGTH + 1];
 	char mnc[OFONO_MAX_MNC_LENGTH + 1];
@@ -171,6 +178,10 @@ static gint spdi_operator_compare(gconstpointer a, gconstpointer b)
 	return strcmp(opa->mnc, opb->mnc);
 }
 
+struct sim_spdi {
+	GSList *operators;
+};
+
 struct sim_spdi *sim_spdi_new(const guint8 *tlv, int length)
 {
 	const guint8 *plmn_list;
@@ -290,11 +301,11 @@ static struct opl_operator *opl_operator_alloc(const guint8 *record)
 }
 
 void sim_eons_add_opl_record(struct sim_eons *eons,
-				const guint8 *tlv, int length)
+				const guint8 *contents, int length)
 {
 	struct opl_operator *oper;
 
-	oper = opl_operator_alloc(tlv);
+	oper = opl_operator_alloc(contents);
 
 	if (oper->id > eons->pnn_max) {
 		g_free(oper);
@@ -324,7 +335,7 @@ void sim_eons_free(struct sim_eons *eons)
 	g_free(eons);
 }
 
-static struct sim_eons_operator_info *
+static const struct sim_eons_operator_info *
 	sim_eons_lookup_common(struct sim_eons *eons,
 				const char *mcc, const char *mnc,
 				gboolean have_lac, guint16 lac)
@@ -372,14 +383,15 @@ static struct sim_eons_operator_info *
 	return &eons->pnn_list[opl->id - 1];
 }
 
-struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
+const struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
 						const char *mcc,
 						const char *mnc)
 {
 	return sim_eons_lookup_common(eons, mcc, mnc, FALSE, 0);
 }
 
-struct sim_eons_operator_info *sim_eons_lookup_with_lac(struct sim_eons *eons,
+const struct sim_eons_operator_info *sim_eons_lookup_with_lac(
+							struct sim_eons *eons,
 							const char *mcc,
 							const char *mnc,
 							guint16 lac)
diff --git a/src/simutil.h b/src/simutil.h
index fb0a1b8..d74706a 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -30,10 +30,6 @@ enum sim_fileid {
 #define SIM_EFSPN_DC_HOME_PLMN_BIT 0x1
 #define SIM_EFSPN_DC_ROAMING_SPN_BIT 0x2
 
-struct sim_spdi {
-	GSList *operators;
-};
-
 struct sim_eons_operator_info {
 	char *longname;
 	gboolean long_ci;
@@ -42,25 +38,19 @@ struct sim_eons_operator_info {
 	char *info;
 };
 
-struct sim_eons {
-	struct sim_eons_operator_info *pnn_list;
-	GSList *opl_list;
-	gboolean pnn_valid;
-	int pnn_max;
-};
-
 struct sim_eons *sim_eons_new(int pnn_records);
 void sim_eons_add_pnn_record(struct sim_eons *eons, int record,
 				const guint8 *tlv, int length);
 gboolean sim_eons_pnn_is_empty(struct sim_eons *eons);
 void sim_eons_add_opl_record(struct sim_eons *eons,
-				const guint8 *tlv, int length);
+				const guint8 *contents, int length);
 void sim_eons_optimize(struct sim_eons *eons);
-struct sim_eons_operator_info *sim_eons_lookup_with_lac(struct sim_eons *eons,
-							const char *mcc,
-							const char *mnc,
-							guint16 lac);
-struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
+const struct sim_eons_operator_info *sim_eons_lookup_with_lac(
+						struct sim_eons *eons,
+						const char *mcc,
+						const char *mnc,
+						guint16 lac);
+const struct sim_eons_operator_info *sim_eons_lookup(struct sim_eons *eons,
 						const char *mcc,
 						const char *mnc);
 void sim_eons_free(struct sim_eons *eons);
-- 
1.6.0


[-- Attachment #4: 0003-Don-t-dereference-before-the-null-check.patch --]
[-- Type: application/octet-stream, Size: 1276 bytes --]

From b41395c24f29b0b68526fab348dce45d8838b7aa Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 18 Jul 2009 01:44:54 +0200
Subject: [PATCH] Don't dereference before the null check.

---
 src/network.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/network.c b/src/network.c
index 969c637..146b49f 100644
--- a/src/network.c
+++ b/src/network.c
@@ -339,7 +339,7 @@ static char *get_operator_display_name(struct ofono_modem *modem)
 {
 	struct network_registration_data *netreg = modem->network_registration;
 	struct network_operator_data *current = netreg->current_operator;
-	struct ofono_network_operator *op = current->info;
+	struct ofono_network_operator *op;
 	const char *plmn;
 	static char name[1024];
 	int len = sizeof(name);
@@ -349,11 +349,13 @@ static char *get_operator_display_name(struct ofono_modem *modem)
 	 * PLMN or roaming and on configuration bits from the SIM, all
 	 * together there are four cases to consider.  */
 
-	if (!netreg->current_operator) {
+	if (!current) {
 		g_strlcpy(name, "", len);
 		return name;
 	}
 
+	op = current->info;
+
 	plmn = op->name;
 	if (current->eons_info && current->eons_info->longname)
 		plmn = current->eons_info->longname;
-- 
1.6.0


[-- Attachment #5: 0004-Use-registration-status-constants-consistently.patch --]
[-- Type: application/octet-stream, Size: 1665 bytes --]

From 9efef8f3481382e6ccd8c8f488d7b73bb6a54087 Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 18 Jul 2009 01:45:22 +0200
Subject: [PATCH] Use registration status constants consistently.

---
 src/network.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/network.c b/src/network.c
index 146b49f..1152b28 100644
--- a/src/network.c
+++ b/src/network.c
@@ -954,7 +954,8 @@ void ofono_network_registration_notify(struct ofono_modem *modem, int status,
 	if (netreg->technology != tech)
 		set_registration_technology(modem, tech);
 
-	if (netreg->status == 1 || netreg->status == 5) {
+	if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED ||
+		netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) {
 		if (netreg->ops->current_operator)
 			netreg->ops->current_operator(modem,
 					current_operator_callback, modem);
@@ -1142,13 +1143,16 @@ static void init_registration_status(const struct ofono_error *error,
 	/* Bootstrap our signal strength value without waiting for the
 	 * stack to report it
 	 */
-	if (netreg->status == 1 || netreg->status == 5) {
+	if (netreg->status == NETWORK_REGISTRATION_STATUS_REGISTERED ||
+		netreg->status == NETWORK_REGISTRATION_STATUS_ROAMING) {
 		if (netreg->ops->signal_strength)
 			netreg->ops->signal_strength(modem,
 					signal_strength_callback, modem);
 	}
 
-	if (AUTO_REGISTER && (status == 0 || status == 3))
+	if (AUTO_REGISTER &&
+		(status == NETWORK_REGISTRATION_STATUS_NOT_REGISTERED ||
+		 status == NETWORK_REGISTRATION_STATUS_DENIED))
 		netreg->ops->register_auto(modem, register_callback, modem);
 }
 
-- 
1.6.0


[-- Attachment #6: 0005-Test-EONS-handling.patch --]
[-- Type: application/octet-stream, Size: 3590 bytes --]

From 4ad1330bb21c76a1902fec21be54422908ca88bc Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 18 Jul 2009 03:29:57 +0200
Subject: [PATCH] Test EONS handling.

---
 unit/Makefile.am    |    5 ++-
 unit/test-simutil.c |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletions(-)
 create mode 100644 unit/test-simutil.c

diff --git a/unit/Makefile.am b/unit/Makefile.am
index a1c5510..d996465 100644
--- a/unit/Makefile.am
+++ b/unit/Makefile.am
@@ -1,5 +1,5 @@
 
-noinst_PROGRAMS = test-common test-util test-sms
+noinst_PROGRAMS = test-common test-util test-sms test-simutil
 
 test_common_SOURCES = test-common.c $(top_srcdir)/src/common.c
 
@@ -8,6 +8,9 @@ test_util_SOURCES = test-util.c $(top_srcdir)/src/util.c
 test_sms_SOURCES = test-sms.c $(top_srcdir)/src/util.c \
 					$(top_srcdir)/src/smsutil.c
 
+test_simutil_SOURCES = test-simutil.c $(top_srcdir)/src/util.c \
+					$(top_srcdir)/src/simutil.c
+
 LDADD = @GLIB_LIBS@ @GTHREAD_LIBS@
 
 AM_CFLAGS = @GTHREAD_CFLAGS@ @GLIB_CFLAGS@
diff --git a/unit/test-simutil.c b/unit/test-simutil.c
new file mode 100644
index 0000000..5945af0
--- /dev/null
+++ b/unit/test-simutil.c
@@ -0,0 +1,79 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2009  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include <assert.h>
+#include <glib.h>
+
+#include "simutil.h"
+
+const unsigned char valid_efopl[] = {
+	0x42, 0xf6, 0x1d, 0x00, 0x00, 0xff, 0xfe, 0x01,
+};
+
+const unsigned char valid_efpnn[][28] = {
+	{ 0x43, 0x0a, 0x00, 0x54, 0x75, 0x78, 0x20, 0x43, 0x6f, 0x6d,
+	  0x6d, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, },
+	{ 0x43, 0x05, 0x00, 0x4C, 0x6F, 0x6E, 0x67, 0x45, 0x06, 0x00,
+	  0x53, 0x68, 0x6F, 0x72, 0x74, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, }
+};
+
+static void test_eons()
+{
+	const struct sim_eons_operator_info *op_info;
+	struct sim_eons *eons_info;
+
+	eons_info = sim_eons_new(2);
+
+	g_assert(sim_eons_pnn_is_empty(eons_info));
+
+	sim_eons_add_pnn_record(eons_info, 1,
+			valid_efpnn[0], sizeof(valid_efpnn[0]));
+	g_assert(!sim_eons_pnn_is_empty(eons_info));
+
+	sim_eons_add_pnn_record(eons_info, 2,
+			valid_efpnn[1], sizeof(valid_efpnn[1]));
+	g_assert(!sim_eons_pnn_is_empty(eons_info));
+
+	sim_eons_add_opl_record(eons_info, valid_efopl, sizeof(valid_efopl));
+	sim_eons_optimize(eons_info);
+
+	op_info = sim_eons_lookup(eons_info, "246", "82");
+	g_assert(!op_info);
+	op_info = sim_eons_lookup(eons_info, "246", "81");
+	g_assert(op_info);
+
+	g_assert(!strcmp(op_info->longname, "Tux Comm"));
+	g_assert(!op_info->shortname);
+	g_assert(!op_info->info);
+
+	sim_eons_free(eons_info);
+}
+
+int main(int argc, char **argv)
+{
+	g_test_init(&argc, &argv, NULL);
+
+	g_test_add_func("/testsimutil/EONS Handling", test_eons);
+
+	return g_test_run();
+}
-- 
1.6.0

Re: [PATCH] Read EF-PNN, EF-OPL and override network names accordingly.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Hi,

> 2009/7/14 Denis Kenzior <denkenz@gmail.com>:
> > Patch has been applied.  However, I've completely (juding by 40 or so
> > commits) refactored how SIM filesystem is handled in oFono so I might
> > have introduced some bugs.  Could you test to make sure it is working as
> > intended?
>
> Attaching some fixes for the new code, the test and a clean-up patch
> (feel free to drop parts).  Note all the recent commits broke
> bisectability badly.

Good catches.  All five have been applied and pushed.  Thanks.

>
> Wouldn't it be useful for the service provider name to be available as
> SimManager property?

I really don't see any practical uses for it by the UI.  Can you foresee 
anything that actually needs this?  Remember, it is easier to add apis later 
than to modify or remove.

Regards,
-Denis