[PATCH] osmo-mgw[master]: WIP: MGCP: Connection Identifiers are hex strings

[Harald Welte]

Review at  https://gerrit.osmocom.org/4906

WIP: MGCP: Connection Identifiers are hex strings

The MGCP spec in RFC3435 is quite clear: Connection Identifiers are
hexadecimal strings of up to 32 characters.  We should not print
and parse them as integers on either client or server.

As we use an uint32_t to represent connection identifiers, we should
use %08x format strings in all places.  As the MGW allocates them,
it doesn't need to support any larger-than-32bit numbers.

However, libosmo-mgcp-client cannot make any assumptions about the
connection ID being an uint32_t or even an uint64_t.  It should
simply treat it as an opaque string.

Closes: OS#2649
Change-Id: I0531a1b670d00cec50078423a2868207135b2436
---
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp_client/mgcp_client_test.c
5 files changed, 9 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/06/4906/1

diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 1a6cbce..168f412 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -63,7 +63,7 @@
 	uint32_t presence;
 	char endpoint[MGCP_ENDPOINT_MAXLEN];
 	unsigned int call_id;
-	uint32_t conn_id;
+	char *conn_id;
 	uint16_t audio_port;
 	char *audio_ip;
 	enum mgcp_connection_mode conn_mode;
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index 726ac63..3b0cba7 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -292,6 +292,7 @@
 		LOGP(DLMGCP, LOGL_ERROR, "Cannot parse MGCP response\n");
 		return -1;
 	}
+	/* FIXME: parse response parameters such as connection ID in case of CRCX! */
 
 	pending = mgcp_client_response_pending_get(mgcp, &r);
 	if (!pending) {
@@ -686,7 +687,7 @@
 
 	/* Add connection id */
 	if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID)
-		rc += msgb_printf(msg, "I: %u\r\n", mgcp_msg->conn_id);
+		rc += msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id);
 
 	/* Add local connection options */
 	if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 763a5a1..0ec4180 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -337,13 +337,13 @@
 	if (!endp)
 		return -1;
 
-	id = strtoul(ci, NULL, 10);
+	id = strtoul(ci, NULL, 16);
 
 	if (mgcp_conn_get(endp, id))
 		return 0;
 
 	LOGP(DLMGCP, LOGL_ERROR,
-	     "endpoint:%x No connection found under ConnectionIdentifier %u\n",
+	     "endpoint:%x No connection found under ConnectionIdentifier %08x\n",
 	     ENDPOINT_NUMBER(endp), id);
 
 	return -1;
@@ -399,7 +399,7 @@
 	if (!ci)
 		return -1;
 
-	*conn_id = strtoul(ci, NULL, 10);
+	*conn_id = strtoul(ci, NULL, 16);
 
 	return 0;
 }
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 2bf8847..80ef3eb 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -222,7 +222,7 @@
 		osmux_extension[0] = '\0';
 	}
 
-	rc = msgb_printf(sdp, "I: %u%s\n\n", conn->conn->id, osmux_extension);
+	rc = msgb_printf(sdp, "I: %08x%s\n\n", conn->conn->id, osmux_extension);
 	if (rc < 0)
 		goto error;
 
@@ -585,7 +585,7 @@
 		return create_err_response(endp, 400, "CRCX", p->trans);
 	}
 
-	snprintf(conn_name, sizeof(conn_name), "%s-%u", callid, conn_id);
+	snprintf(conn_name, sizeof(conn_name), "%s-%08x", callid, conn_id);
 	mgcp_conn_alloc(NULL, endp, conn_id, MGCP_CONN_TYPE_RTP,
 			conn_name);
 	conn = mgcp_conn_get_rtp(endp, conn_id);
diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c
index 513f345..5fd59e9 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -162,7 +162,7 @@
 		.endpoint = "23 at mgw",
 		.audio_port = 1234,
 		.call_id = 47,
-		.conn_id = 11,
+		.conn_id = "11",
 		.conn_mode = MGCP_CONN_RECV_SEND
 	};
 

-- 
To view, visit https://gerrit.osmocom.org/4906
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0531a1b670d00cec50078423a2868207135b2436
Gerrit-PatchSet: 1
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>