Re: MNC/MCC as string?

[Aki Niemi <aki@protocolpolice.com>]

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

On Fri, 12 Jun 2009 12:12:33 +0200, Marcel Holtmann <marcel@holtmann.org>
wrote:
> +       strncpy(mcc, str, sizeof(mcc));
> +       mcc[3] = '\0';
> 
> maybe I am blind, but how is this suppose to work. The sizeof(mmc) is 1
> byte.

Actually, sizeof(mmc) is 4 bytes. Coincidentally, so is the char array in
mcc. ;)

Take two attached.

Cheers,
Aki

[-- Attachment #2: 0001-Change-MNC-and-MCC-variable-types-to-string.patch --]
[-- Type: text/plain, Size: 8689 bytes --]

From 5ea55baef4a10b3977675284c83d21e0a8f54a7b Mon Sep 17 00:00:00 2001
From: Aki Niemi <aki.niemi@nokia.com>
Date: Fri, 12 Jun 2009 10:02:52 +0300
Subject: [PATCH] Change MNC and MCC variable types to string

This is to make sure both 2 and 3-digit MNC values are correctly
handled. Both the modem plugin API as well as the D-Bus API are
affected.
---
 drivers/atmodem/network-registration.c |   57 +++++++++++++++-----------------
 src/driver.h                           |    7 +++-
 src/network.c                          |   48 ++++++++++----------------
 3 files changed, 51 insertions(+), 61 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 40796d5..30dd0f7 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -45,28 +45,20 @@ static const char *csq_prefix[] = { "+CSQ:", NULL };
 
 struct netreg_data {
 	gboolean supports_tech;
-	short mnc;
-	short mcc;
+	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
 };
 
-static void extract_mcc_mnc(const char *str, short *mcc, short *mnc)
+static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
 {
-	int num = 0;
-	unsigned int i;
-
 	/* Three digit country code */
-	for (i = 0; i < 3; i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mcc = num;
-
-	num = 0;
+	strncpy(mcc, str, OFONO_MAX_MNC_MCC_LENGTH);
+	mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
 
 	/* Usually a 2 but sometimes 3 digit network code */
-	for (; i < strlen(str); i++)
-		num = num * 10 + (int)(str[i] - '0');
-
-	*mnc = num;
+	strncpy(mnc, str + OFONO_MAX_MNC_MCC_LENGTH,
+		OFONO_MAX_MNC_MCC_LENGTH);
+	mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
 }
 
 static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -154,7 +146,7 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	dump_response("cops_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
 
-	if (!ok || at->netreg->mcc == -1 || at->netreg->mnc == -1) {
+	if (!ok || *at->netreg->mcc == '\0' || *at->netreg->mnc == '\0') {
 		cb(&error, NULL, cbd->data);
 		goto out;
 	}
@@ -181,12 +173,16 @@ static void cops_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	strncpy(op.name, name, OFONO_MAX_OPERATOR_NAME_LENGTH);
 	op.name[OFONO_MAX_OPERATOR_NAME_LENGTH] = '\0';
 
-	op.mcc = at->netreg->mcc;
-	op.mnc = at->netreg->mnc;
+	strncpy(op.mcc, at->netreg->mcc, OFONO_MAX_MNC_MCC_LENGTH);
+	op.mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+
+	strncpy(op.mnc, at->netreg->mnc, OFONO_MAX_MNC_MCC_LENGTH);
+	op.mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+
 	op.status = -1;
 	op.tech = tech;
 
-	ofono_debug("cops_cb: %s, %hd %hd %d", name, at->netreg->mcc,
+	ofono_debug("cops_cb: %s, %s %s %d", name, at->netreg->mcc,
 			at->netreg->mnc, tech);
 
 	cb(&error, &op, cbd->data);
@@ -235,15 +231,16 @@ static void cops_numeric_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		strlen(str) == 0)
 		goto error;
 
-	extract_mcc_mnc(str, &at->netreg->mcc, &at->netreg->mnc);
+	extract_mcc_mnc(str, at->netreg->mcc, at->netreg->mnc);
 
-	ofono_debug("Cops numeric got mcc: %hd, mnc: %hd",
+	ofono_debug("Cops numeric got mcc: %s, mnc: %s",
 			at->netreg->mcc, at->netreg->mnc);
 
 	return;
 
 error:
-	at->netreg->mcc = at->netreg->mnc = -1;
+	*at->netreg->mcc = '\0';
+	*at->netreg->mnc = '\0';
 }
 
 static void at_current_operator(struct ofono_modem *modem,
@@ -356,7 +353,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 			if (!g_at_result_iter_next_string(&iter, &n))
 				break;
 
-			extract_mcc_mnc(n, &list[num].mcc, &list[num].mnc);
+			extract_mcc_mnc(n, list[num].mcc, list[num].mnc);
 
 			if (!g_at_result_iter_next_number(&iter, &tech))
 				tech = 0;
@@ -376,7 +373,7 @@ static void cops_list_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	int i = 0;
 
 	for (; i < num; i++) {
-		ofono_debug("Operator: %s, %hd, %hd, status: %d, %d",
+		ofono_debug("Operator: %s, %s, %s, status: %d, %d",
 				list[i].name, list[i].mcc, list[i].mnc,
 				list[i].status, list[i].tech);
 	}
@@ -457,12 +454,12 @@ static void at_register_manual(struct ofono_modem *modem,
 		goto error;
 
 	if (at->netreg->supports_tech && oper->tech != -1)
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\",%1d", oper->mcc,
-							oper->mnc,
-							oper->tech);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\",%1d", oper->mcc,
+						oper->mnc,
+						oper->tech);
 	else
-		sprintf(buf, "AT+COPS=1,2,\"%03hd%02hd\"", oper->mcc,
-							oper->mnc);
+		sprintf(buf, "AT+COPS=1,2,\"%s%s\"", oper->mcc,
+						oper->mnc);
 
 	if (g_at_chat_send(at->parser, buf, none_prefix,
 				register_cb, cbd, g_free) > 0)
diff --git a/src/driver.h b/src/driver.h
index 61504dd..f753c7f 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -74,10 +74,13 @@ struct ofono_call {
  *   */
 #define OFONO_MAX_OPERATOR_NAME_LENGTH 63
 
+/* MCC is always three digits. MNC is either two or three digits */
+#define OFONO_MAX_MNC_MCC_LENGTH 3
+
 struct ofono_network_operator {
 	char name[OFONO_MAX_OPERATOR_NAME_LENGTH + 1];
-	short mcc;
-	short mnc;
+	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
 	int status;
 	int tech;
 };
diff --git a/src/network.c b/src/network.c
index f797165..aa13077 100644
--- a/src/network.c
+++ b/src/network.c
@@ -180,8 +180,9 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 	int modem_len;
 	int num_children;
 	GSList *l;
-	int *mccmnc;
 	char path[MAX_DBUS_PATH_LEN];
+	char mnc[4];
+	char mcc[4];
 
 	modem_len = snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator",
 				modem->path);
@@ -199,10 +200,6 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 
 	*network_operators = g_try_new0(char *, num_children + 1);
 
-	mccmnc = g_try_new0(int, num_children * 2);
-	for (i = 0; i < num_children; i++)
-		sscanf(children[i], "%3d%3d", &mccmnc[i*2], &mccmnc[i*2+1]);
-
 	/* Quoting 27.007: "The list of operators shall be in order: home
 	 * network, networks referenced in SIM or active application in the
 	 * UICC (GSM or USIM) in the following order: HPLMN selector, User
@@ -217,18 +214,17 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 		int j;
 
 		for (j = 0; children[j]; j++) {
-			if (op->mcc == mccmnc[j*2] && op->mnc == mccmnc[j*2+1]) {
-				/* Enough to store '/' + 3 char wide MCC + 3 char wide MNC + null */
-				(*network_operators)[i] =  g_try_new(char, modem_len + 8);
-				snprintf((*network_operators)[i], modem_len + 8, "%s/%s",
+			sscanf(children[j], "%3[0-9]%[0-9]", mcc, mnc);
+			if (strcmp(op->mcc, mcc) == 0 && strcmp(op->mnc, mnc) == 0) {
+				/* Enough to store '/' + MCC + '_' + MNC + null */
+				(*network_operators)[i] =  g_try_new(char, modem_len + 9);
+				snprintf((*network_operators)[i], modem_len + 9, "%s/%s",
 					path, children[j]);
 				++i;
 			}
 		}
 	}
 
-	g_free(mccmnc);
-
 	dbus_free_string_array(children);
 }
 
@@ -244,19 +240,13 @@ static gint network_operator_compare(gconstpointer a, gconstpointer b)
 	const struct ofono_network_operator *opa = a;
 	const struct ofono_network_operator *opb = b;
 
-	if (opa->mcc < opb->mcc)
-		return -1;
+	int comp1;
+	int comp2;
 
-	if (opa->mcc > opb->mcc)
-		return 1;
+	comp1 = strcmp(opa->mcc, opb->mcc);
+	comp2 = strcmp(opa->mnc, opb->mnc);
 
-	if (opa->mnc < opb->mnc)
-		return -1;
-
-	if (opa->mnc > opb->mnc)
-		return 1;
-
-	return 0;
+	return comp1 != 0 ? comp1 : comp2;
 }
 
 static inline const char *network_operator_build_path(struct ofono_modem *modem,
@@ -264,7 +254,7 @@ static inline const char *network_operator_build_path(struct ofono_modem *modem,
 {
 	static char path[MAX_DBUS_PATH_LEN];
 
-	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%03d%03d",
+	snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator/%s%s",
 			modem->path, oper->mcc, oper->mnc);
 
 	return path;
@@ -427,16 +417,16 @@ static DBusMessage *network_operator_get_properties(DBusConnection *conn,
 
 	dbus_gsm_dict_append(&dict, "Status", DBUS_TYPE_STRING, &status);
 
-	if (op->operator->mcc != -1) {
-		dbus_uint16_t mcc = op->operator->mcc;
+	if (*op->operator->mcc != '\0') {
+		const char *mcc = op->operator->mcc;
 		dbus_gsm_dict_append(&dict, "MobileCountryCode",
-					DBUS_TYPE_UINT16, &mcc);
+					DBUS_TYPE_STRING, &mcc);
 	}
 
-	if (op->operator->mnc != -1) {
-		dbus_uint16_t mnc = op->operator->mnc;
+	if (*op->operator->mnc != '\0') {
+		const char *mnc = op->operator->mnc;
 		dbus_gsm_dict_append(&dict, "MobileNetworkCode",
-					DBUS_TYPE_UINT16, &mnc);
+					DBUS_TYPE_STRING, &mnc);
 	}
 
 	if (op->operator->tech != -1) {
-- 
1.6.0.4