Fixup style issues with previous MCC/MNC patch

- Breakup MCC/MNC LENGTH constant
- Don't hardcode numbers
- Fix >80 column length
- Fix test case to expect strings instead of shorts

4 files changed, 35 insertions, 32 deletions

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 30dd0f7e..f30c41af 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -45,20 +45,19 @@ static const char *csq_prefix[] = { "+CSQ:", NULL };
 
 struct netreg_data {
 	gboolean supports_tech;
-	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
-	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mcc[OFONO_MAX_MCC_LENGTH + 1];
+	char mnc[OFONO_MAX_MNC_LENGTH + 1];
 };
 
 static void extract_mcc_mnc(const char *str, char *mcc, char *mnc)
 {
 	/* Three digit country code */
-	strncpy(mcc, str, OFONO_MAX_MNC_MCC_LENGTH);
-	mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+	strncpy(mcc, str, OFONO_MAX_MCC_LENGTH);
+	mcc[OFONO_MAX_MCC_LENGTH] = '\0';
 
 	/* Usually a 2 but sometimes 3 digit network code */
-	strncpy(mnc, str + OFONO_MAX_MNC_MCC_LENGTH,
-		OFONO_MAX_MNC_MCC_LENGTH);
-	mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+	strncpy(mnc, str + OFONO_MAX_MCC_LENGTH, OFONO_MAX_MNC_LENGTH);
+	mnc[OFONO_MAX_MNC_LENGTH] = '\0';
 }
 
 static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -146,7 +145,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 == '\0' || *at->netreg->mnc == '\0') {
+	if (!ok || at->netreg->mcc[0] == '\0' || at->netreg->mnc[0] == '\0') {
 		cb(&error, NULL, cbd->data);
 		goto out;
 	}
@@ -173,11 +172,11 @@ 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';
 
-	strncpy(op.mcc, at->netreg->mcc, OFONO_MAX_MNC_MCC_LENGTH);
-	op.mcc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+	strncpy(op.mcc, at->netreg->mcc, OFONO_MAX_MCC_LENGTH);
+	op.mcc[OFONO_MAX_MCC_LENGTH] = '\0';
 
-	strncpy(op.mnc, at->netreg->mnc, OFONO_MAX_MNC_MCC_LENGTH);
-	op.mnc[OFONO_MAX_MNC_MCC_LENGTH] = '\0';
+	strncpy(op.mnc, at->netreg->mnc, OFONO_MAX_MNC_LENGTH);
+	op.mnc[OFONO_MAX_MNC_LENGTH] = '\0';
 
 	op.status = -1;
 	op.tech = tech;
@@ -239,8 +238,8 @@ static void cops_numeric_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	return;
 
 error:
-	*at->netreg->mcc = '\0';
-	*at->netreg->mnc = '\0';
+	at->netreg->mcc[0] = '\0';
+	at->netreg->mnc[0] = '\0';
 }
 
 static void at_current_operator(struct ofono_modem *modem,
@@ -454,12 +453,10 @@ 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,\"%s%s\",%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,\"%s%s\"", 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 f753c7f5..c9281795 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -75,12 +75,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
+#define OFONO_MAX_MCC_LENGTH 3
+#define OFONO_MAX_MNC_LENGTH 3
 
 struct ofono_network_operator {
 	char name[OFONO_MAX_OPERATOR_NAME_LENGTH + 1];
-	char mcc[OFONO_MAX_MNC_MCC_LENGTH + 1];
-	char mnc[OFONO_MAX_MNC_MCC_LENGTH + 1];
+	char mcc[OFONO_MAX_MCC_LENGTH + 1];
+	char mnc[OFONO_MAX_MNC_LENGTH + 1];
 	int status;
 	int tech;
 };
diff --git a/src/network.c b/src/network.c
index aa13077f..a437f0dd 100644
--- a/src/network.c
+++ b/src/network.c
@@ -177,14 +177,15 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 	DBusConnection *conn = dbus_gsm_connection();
 	char **children;
 	int i;
-	int modem_len;
+	int prefix_len;
 	int num_children;
 	GSList *l;
 	char path[MAX_DBUS_PATH_LEN];
-	char mnc[4];
-	char mcc[4];
+	char mnc[OFONO_MAX_MNC_LENGTH + 1];
+	char mcc[OFONO_MAX_MCC_LENGTH + 1];
+	int op_path_len;
 
-	modem_len = snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator",
+	prefix_len = snprintf(path, MAX_DBUS_PATH_LEN, "%s/operator",
 				modem->path);
 
 	if (!dbus_connection_list_registered(conn, path, &children)) {
@@ -199,6 +200,10 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 	num_children = i;
 
 	*network_operators = g_try_new0(char *, num_children + 1);
+	
+	/* Enough to store '/' + MCC + MNC + null */
+	op_path_len = prefix_len;
+	op_path_len += OFONO_MAX_MCC_LENGTH + OFONO_MAX_MNC_LENGTH + 2;
 
 	/* Quoting 27.007: "The list of operators shall be in order: home
 	 * network, networks referenced in SIM or active application in the
@@ -215,11 +220,11 @@ static void network_operator_populate_registered(struct ofono_modem *modem,
 
 		for (j = 0; children[j]; j++) {
 			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]);
+			if (!strcmp(op->mcc, mcc) && !strcmp(op->mnc, mnc)) {
+				(*network_operators)[i] =
+					g_try_new(char, op_path_len);
+				snprintf((*network_operators)[i], op_path_len,
+						"%s/%s", path, children[j]);
 				++i;
 			}
 		}
diff --git a/test/test-network-registration b/test/test-network-registration
index 8d2176b5..5bc713a0 100755
--- a/test/test-network-registration
+++ b/test/test-network-registration
@@ -66,7 +66,7 @@ if __name__ == "__main__":
 		print ""
 
 		print "Operator at path: '%s'" % (path)
-		print "Name: '%s', MCC: '%d', MNC: '%d', status: '%s'" %\
+		print "Name: '%s', MCC: '%s', MNC: '%s', status: '%s'" %\
 			(props['Name'], props['MobileCountryCode'],
 				props['MobileNetworkCode'], props['Status'])