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

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Fri Nov 24 11:44:12 UTC 2017


Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/4906

to look at the new patch set (#3).

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.

Change the internal uint32_t representation of connection identifiers
to a string representation in the client and also in the server.

Closes: OS#2649
Change-Id: I0531a1b670d00cec50078423a2868207135b2436
---
M TODO-RELEASE
M include/osmocom/mgcp/mgcp_common.h
M include/osmocom/mgcp/mgcp_conn.h
M include/osmocom/mgcp/mgcp_internal.h
M include/osmocom/mgcp/mgcp_msg.h
M include/osmocom/mgcp_client/mgcp_client.h
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp/mgcp_conn.c
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_network.c
M src/libosmo-mgcp/mgcp_osmux.c
M src/libosmo-mgcp/mgcp_protocol.c
M src/libosmo-mgcp/mgcp_sdp.c
M src/libosmo-mgcp/mgcp_vty.c
M tests/mgcp/mgcp_test.c
M tests/mgcp_client/mgcp_client_test.c
16 files changed, 100 insertions(+), 102 deletions(-)


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

diff --git a/TODO-RELEASE b/TODO-RELEASE
index d198b97..917c995 100644
--- a/TODO-RELEASE
+++ b/TODO-RELEASE
@@ -24,3 +24,4 @@
 # If any interfaces have been removed or changed since the last public release, a=0.
 #
 #library	what		description / commit summary line
+libosmo-mgcp	API/ABI change	parse and represent connection identifiers as hex strings
\ No newline at end of file
diff --git a/include/osmocom/mgcp/mgcp_common.h b/include/osmocom/mgcp/mgcp_common.h
index 0eb1388..5df7ab0 100644
--- a/include/osmocom/mgcp/mgcp_common.h
+++ b/include/osmocom/mgcp/mgcp_common.h
@@ -68,4 +68,8 @@
 	return 0;
 }
 
+/* String length of Connection Identifiers
+ * (see also RFC3435 2.1.3.2 Names of Connections) */
+#define MGCP_CONN_ID_MAXLEN 32+1
+
 #endif
diff --git a/include/osmocom/mgcp/mgcp_conn.h b/include/osmocom/mgcp/mgcp_conn.h
index e0ae021..982a311 100644
--- a/include/osmocom/mgcp/mgcp_conn.h
+++ b/include/osmocom/mgcp/mgcp_conn.h
@@ -28,12 +28,12 @@
 #include <inttypes.h>
 
 struct mgcp_conn *mgcp_conn_alloc(void *ctx, struct mgcp_endpoint *endp,
-				  uint32_t id, enum mgcp_conn_type type,
+				  const char *id, enum mgcp_conn_type type,
 				  char *name);
-struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, uint32_t id);
+struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id);
 struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp,
-					uint32_t id);
-void mgcp_conn_free(struct mgcp_endpoint *endp, uint32_t id);
+					const char *id);
+void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id);
 void mgcp_conn_free_oldest(struct mgcp_endpoint *endp);
 void mgcp_conn_free_all(struct mgcp_endpoint *endp);
 char *mgcp_conn_dump(struct mgcp_conn *conn);
diff --git a/include/osmocom/mgcp/mgcp_internal.h b/include/osmocom/mgcp/mgcp_internal.h
index b9c1731..cb2e796 100644
--- a/include/osmocom/mgcp/mgcp_internal.h
+++ b/include/osmocom/mgcp/mgcp_internal.h
@@ -30,8 +30,10 @@
 
 #define CI_UNUSED 0
 
-#define CONN_ID_BTS 0
-#define CONN_ID_NET 1
+/* FIXME: This this is only needed to compile the currently
+ * broken OSMUX support. Remove when fixed */
+#define CONN_ID_BTS "0"
+#define CONN_ID_NET "1"
 
 enum mgcp_trunk_type {
 	MGCP_TRUNK_VIRTUAL,
@@ -203,7 +205,7 @@
 	enum mgcp_connection_mode mode_orig;
 
 	/*!< connection id to identify the conntion */
-	uint32_t id;
+	char id[MGCP_CONN_ID_MAXLEN];
 
 	/*!< human readable name (vty, logging) */
 	char name[256];
diff --git a/include/osmocom/mgcp/mgcp_msg.h b/include/osmocom/mgcp/mgcp_msg.h
index b7d52bb..7732865 100644
--- a/include/osmocom/mgcp/mgcp_msg.h
+++ b/include/osmocom/mgcp/mgcp_msg.h
@@ -43,7 +43,7 @@
 
 int mgcp_verify_call_id(struct mgcp_endpoint *endp, const char *callid);
 
-int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *ci);
+int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *conn_id);
 
 char *mgcp_strline(char *str, char **saveptr);
 
@@ -54,5 +54,3 @@
 #define for_each_non_empty_line(line, save)\
 	for (line = strtok_r(NULL, "\r\n", &save); line;\
 	     line = strtok_r(NULL, "\r\n", &save))
-
-int mgcp_parse_ci(uint32_t *conn_id, const char *ci);
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index 1a6cbce..919da68 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -30,6 +30,7 @@
        int response_code;
        mgcp_trans_id_t trans_id;
        const char *comment;
+	char conn_id[MGCP_CONN_ID_MAXLEN];
 };
 
 struct mgcp_response {
@@ -63,7 +64,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 2047637..ad972de 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -718,7 +718,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_conn.c b/src/libosmo-mgcp/mgcp_conn.c
index e07b766..7534d4a 100644
--- a/src/libosmo-mgcp/mgcp_conn.c
+++ b/src/libosmo-mgcp/mgcp_conn.c
@@ -78,11 +78,13 @@
  *  \param[in] type connection type (e.g. MGCP_CONN_TYPE_RTP)
  *  \returns pointer to allocated connection, NULL on error */
 struct mgcp_conn *mgcp_conn_alloc(void *ctx, struct mgcp_endpoint *endp,
-				  uint32_t id, enum mgcp_conn_type type,
+				  const char *id, enum mgcp_conn_type type,
 				  char *name)
 {
 	struct mgcp_conn *conn;
 	OSMO_ASSERT(endp);
+	OSMO_ASSERT(id);
+	OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN);
 	OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
 	OSMO_ASSERT(strlen(name) < sizeof(conn->name));
 
@@ -102,9 +104,9 @@
 	conn->type = type;
 	conn->mode = MGCP_CONN_NONE;
 	conn->mode_orig = MGCP_CONN_NONE;
-	conn->id = id;
 	conn->u.rtp.conn = conn;
 	strcpy(conn->name, name);
+	osmo_strlcpy(conn->id, id, sizeof(conn->id));
 
 	switch (type) {
 	case MGCP_CONN_TYPE_RTP:
@@ -126,15 +128,17 @@
  *  \param[in] endp associated endpoint
  *  \param[in] id identification number of the connection
  *  \returns pointer to allocated connection, NULL if not found */
-struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, uint32_t id)
+struct mgcp_conn *mgcp_conn_get(struct mgcp_endpoint *endp, const char *id)
 {
 	OSMO_ASSERT(endp);
+	OSMO_ASSERT(id);
+	OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN);
 	OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
 
 	struct mgcp_conn *conn;
 
 	llist_for_each_entry(conn, &endp->conns, entry) {
-		if (conn->id == id)
+		if (strncmp(conn->id, id, sizeof(conn->id)) == 0)
 			return conn;
 	}
 
@@ -145,9 +149,12 @@
  *  \param[in] endp associated endpoint
  *  \param[in] id identification number of the connection
  *  \returns pointer to allocated connection, NULL if not found */
-struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp, uint32_t id)
+struct mgcp_conn_rtp *mgcp_conn_get_rtp(struct mgcp_endpoint *endp,
+					const char *id)
 {
 	OSMO_ASSERT(endp);
+	OSMO_ASSERT(id);
+	OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN);
 	OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
 
 	struct mgcp_conn *conn;
@@ -165,9 +172,11 @@
 /*! free a connection by its ID.
  *  \param[in] endp associated endpoint
  *  \param[in] id identification number of the connection */
-void mgcp_conn_free(struct mgcp_endpoint *endp, uint32_t id)
+void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id)
 {
 	OSMO_ASSERT(endp);
+	OSMO_ASSERT(id);
+	OSMO_ASSERT(strlen(id) < MGCP_CONN_ID_MAXLEN);
 	OSMO_ASSERT(endp->conns.next != NULL && endp->conns.prev != NULL);
 
 	struct mgcp_conn *conn;
@@ -235,7 +244,7 @@
  *  \returns human readble string */
 char *mgcp_conn_dump(struct mgcp_conn *conn)
 {
-	static char str[sizeof(conn->name)+256];
+	static char str[sizeof(conn->name)+sizeof(conn->id)+256];
 
 	if (!conn) {
 		snprintf(str, sizeof(str), "(null connection)");
@@ -245,7 +254,7 @@
 	switch (conn->type) {
 	case MGCP_CONN_TYPE_RTP:
 		/* Dump RTP connection */
-		snprintf(str, sizeof(str), "(%s/rtp, id:%u, ip:%s, "
+		snprintf(str, sizeof(str), "(%s/rtp, id:%s, ip:%s, "
 			 "rtp:%u rtcp:%u)",
 			 conn->name,
 			 conn->id,
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index 763a5a1..656733c 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -330,21 +330,39 @@
   * \param[in] endp pointer to endpoint
   * \param{in] connection id to verify
   * \returns 1 when connection id seems plausible, 0 on error */
-int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *ci)
+int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *conn_id)
 {
-	uint32_t id;
-
-	if (!endp)
+	/* Check for null identifiers */
+	if (!conn_id) {
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "endpoint:%x invalid ConnectionIdentifier (missing) %s\n",
+		     ENDPOINT_NUMBER(endp), conn_id);
 		return -1;
+	}
 
-	id = strtoul(ci, NULL, 10);
+	/* Check for empty connection identifiers */
+	if (strlen(conn_id) == 0) {
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "endpoint:%x invalid ConnectionIdentifier (empty) %s\n",
+		     ENDPOINT_NUMBER(endp), conn_id);
+		return -1;
+	}
 
-	if (mgcp_conn_get(endp, id))
+	/* Check for over long connection identifiers */
+	if (strlen(conn_id) > MGCP_CONN_ID_MAXLEN) {
+		LOGP(DLMGCP, LOGL_ERROR,
+		     "endpoint:%x invalid ConnectionIdentifier (too long) %s\n",
+		     ENDPOINT_NUMBER(endp), conn_id);
+		return -1;
+	}
+
+	/* Check if connection exists */
+	if (mgcp_conn_get(endp, conn_id))
 		return 0;
 
 	LOGP(DLMGCP, LOGL_ERROR,
-	     "endpoint:%x No connection found under ConnectionIdentifier %u\n",
-	     ENDPOINT_NUMBER(endp), id);
+	     "endpoint:%x No connection found under ConnectionIdentifier %s\n",
+	     ENDPOINT_NUMBER(endp), conn_id);
 
 	return -1;
 }
@@ -385,21 +403,4 @@
 	}
 
 	return result;
-}
-
-/*! Parse CI from a given string.
-  * \param[out] caller provided memory to store the result
-  * \param{in] string containing the connection id
-  * \returns 0 on success, -1 on error */
-int mgcp_parse_ci(uint32_t *conn_id, const char *ci)
-{
-
-	OSMO_ASSERT(conn_id);
-
-	if (!ci)
-		return -1;
-
-	*conn_id = strtoul(ci, NULL, 10);
-
-	return 0;
 }
diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c
index d51b829..a02b0d1 100644
--- a/src/libosmo-mgcp/mgcp_network.c
+++ b/src/libosmo-mgcp/mgcp_network.c
@@ -73,11 +73,11 @@
 		rc = osmo_sock_local_ip(addr, inet_ntoa(conn->end.addr));
 		if (rc < 0)
 			LOGP(DRTP, LOGL_ERROR,
-			     "endpoint:%x CI:%i local interface auto detection failed, using configured addresses...\n",
+			     "endpoint:%x CI:%s local interface auto detection failed, using configured addresses...\n",
 			     ENDPOINT_NUMBER(endp), conn->conn->id);
 		else {
 			LOGP(DRTP, LOGL_DEBUG,
-			     "endpoint:%x CI:%i selected local rtp bind ip %s by probing using remote ip %s\n",
+			     "endpoint:%x CI:%s selected local rtp bind ip %s by probing using remote ip %s\n",
 			     ENDPOINT_NUMBER(endp), conn->conn->id, addr,
 			     inet_ntoa(conn->end.addr));
 			return;
@@ -90,7 +90,7 @@
 		 * if so, use that IP-Address */
 		strncpy(addr, endp->cfg->net_ports.bind_addr, INET_ADDRSTRLEN);
 		LOGP(DRTP, LOGL_DEBUG,
-		     "endpoint:%x CI:%i using configured rtp bind ip as local bind ip %s\n",
+		     "endpoint:%x CI:%s using configured rtp bind ip as local bind ip %s\n",
 		     ENDPOINT_NUMBER(endp), conn->conn->id, addr);
 	} else {
 		/* No specific bind IP is configured for the RTP traffic, so
@@ -98,7 +98,7 @@
 		 * as bind IP */
 		strncpy(addr, endp->cfg->source_addr, INET_ADDRSTRLEN);
 		LOGP(DRTP, LOGL_DEBUG,
-		     "endpoint:%x CI:%i using mgcp bind ip as local rtp bind ip: %s\n",
+		     "endpoint:%x CI:%s using mgcp bind ip as local rtp bind ip: %s\n",
 		     ENDPOINT_NUMBER(endp), conn->conn->id, addr);
 	}
 }
@@ -1217,7 +1217,7 @@
 	struct mgcp_rtp_end *end;
 	char local_ip_addr[INET_ADDRSTRLEN];
 
-	snprintf(name, sizeof(name), "%s-%u", conn->conn->name, conn->conn->id);
+	snprintf(name, sizeof(name), "%s-%s", conn->conn->name, conn->conn->id);
 	end = &conn->end;
 
 	if (end->rtp.fd != -1 || end->rtcp.fd != -1) {
diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c
index 09b2636..5030812 100644
--- a/src/libosmo-mgcp/mgcp_osmux.c
+++ b/src/libosmo-mgcp/mgcp_osmux.c
@@ -573,7 +573,7 @@
 	if (conn->osmux.state != OSMUX_STATE_ENABLED)
 		return;
 
-	LOGP(DLMGCP, LOGL_INFO, "Releasing connection %u using Osmux CID %u\n",
+	LOGP(DLMGCP, LOGL_INFO, "Releasing connection %s using Osmux CID %u\n",
 	     conn->conn->id, conn->osmux.cid);
 	osmux_xfrm_input_close_circuit(conn->osmux.in, conn->osmux.cid);
 	conn->osmux.state = OSMUX_STATE_DISABLED;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 9e04e50..a326f00 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -221,7 +221,7 @@
 		osmux_extension[0] = '\0';
 	}
 
-	rc = msgb_printf(sdp, "I: %u%s\n\n", conn->conn->id, osmux_extension);
+	rc = msgb_printf(sdp, "I: %s%s\n\n", conn->conn->id, osmux_extension);
 	if (rc < 0)
 		goto error;
 
@@ -443,12 +443,11 @@
 
 	const char *local_options = NULL;
 	const char *callid = NULL;
-	const char *ci = NULL;
 	const char *mode = NULL;
 	char *line;
 	int have_sdp = 0, osmux_cid = -1;
 	struct mgcp_conn_rtp *conn = NULL;
-	uint32_t conn_id;
+        const char *conn_id = NULL;
 	char conn_name[512];
 
 	LOGP(DLMGCP, LOGL_NOTICE, "CRCX: creating new connection ...\n");
@@ -469,7 +468,7 @@
 			callid = (const char *)line + 3;
 			break;
 		case 'I':
-			ci = (const char *)line + 3;
+			conn_id = (const char *)line + 3;
 			break;
 		case 'M':
 			mode = (const char *)line + 3;
@@ -511,7 +510,7 @@
 		return create_err_response(endp, 400, "CRCX", p->trans);
 	}
 
-	if (!ci) {
+	if (!conn_id) {
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "CRCX: endpoint:%x insufficient parameters, missing connection id\n",
 		     ENDPOINT_NUMBER(endp));
@@ -561,13 +560,6 @@
 	set_local_cx_options(endp->tcfg->endpoints, &endp->local_options,
 			     local_options);
 
-	if (mgcp_parse_ci(&conn_id, ci)) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "CRCX: endpoint:%x insufficient parameters, missing ci (connectionIdentifier)\n",
-		     ENDPOINT_NUMBER(endp));
-		return create_err_response(endp, 400, "CRCX", p->trans);
-	}
-
 	/* Only accept another connection when the connection ID is different. */
 	if (mgcp_conn_get_rtp(endp, conn_id)) {
 		LOGP(DLMGCP, LOGL_ERROR,
@@ -583,7 +575,7 @@
 		}
 	}
 
-	snprintf(conn_name, sizeof(conn_name), "%s-%u", callid, conn_id);
+	snprintf(conn_name, sizeof(conn_name), "%s-%s", callid, conn_id);
 	mgcp_conn_alloc(NULL, endp, conn_id, MGCP_CONN_TYPE_RTP,
 			conn_name);
 	conn = mgcp_conn_get_rtp(endp, conn_id);
@@ -664,7 +656,7 @@
 	}
 
 	LOGP(DLMGCP, LOGL_DEBUG,
-	     "CRCX: endpoint:%x Creating connection: CI: %u port: %u\n",
+	     "CRCX: endpoint:%x Creating connection: CI: %s port: %u\n",
 	     ENDPOINT_NUMBER(endp), conn->conn->id, conn->end.local_port);
 	if (p->cfg->change_cb)
 		p->cfg->change_cb(tcfg, ENDPOINT_NUMBER(endp), MGCP_ENDP_CRCX);
@@ -695,11 +687,10 @@
 	int silent = 0;
 	int have_sdp = 0;
 	char *line;
-	const char *ci = NULL;
 	const char *local_options = NULL;
 	const char *mode = NULL;
 	struct mgcp_conn_rtp *conn = NULL;
-	uint32_t conn_id;
+        const char *conn_id = NULL;
 
 	LOGP(DLMGCP, LOGL_NOTICE, "MDCX: modifying existing connection ...\n");
 
@@ -723,8 +714,8 @@
 				goto error3;
 			break;
 		case 'I':
-			ci = (const char *)line + 3;
-			if (mgcp_verify_ci(endp, ci) != 0)
+			conn_id = (const char *)line + 3;
+			if (mgcp_verify_ci(endp, conn_id) != 0)
 				goto error3;
 			break;
 		case 'L':
@@ -749,7 +740,7 @@
 	}
 
 mgcp_header_done:
-	if (mgcp_parse_ci(&conn_id, ci)) {
+	if (!conn_id) {
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "MDCX: endpoint:%x insufficient parameters, missing ci (connectionIdentifier)\n",
 		     ENDPOINT_NUMBER(endp));
@@ -849,9 +840,8 @@
 	int silent = 0;
 	char *line;
 	char stats[1048];
-	const char *ci = NULL;
+	const char *conn_id = NULL;
 	struct mgcp_conn_rtp *conn = NULL;
-	uint32_t conn_id;
 
 	if (p->found != 0)
 		return create_err_response(NULL, error_code, "DLCX", p->trans);
@@ -877,8 +867,8 @@
 				goto error3;
 			break;
 		case 'I':
-			ci = (const char *)line + 3;
-			if (mgcp_verify_ci(endp, ci) != 0)
+			conn_id = (const char *)line + 3;
+			if (mgcp_verify_ci(endp, conn_id) != 0)
 				goto error3;
 			break;
 		case 'Z':
@@ -919,7 +909,7 @@
 	/* When no connection id is supplied, we will interpret this as a
 	 * wildcarded DLCX and drop all connections at once. (See also
 	 * RFC3435 Section F.7) */
-	if (!ci) {
+	if (!conn_id) {
 		LOGP(DLMGCP, LOGL_NOTICE,
 		     "DLCX: endpoint:%x missing ci (connectionIdentifier), will remove all connections at once\n",
 		     ENDPOINT_NUMBER(endp));
@@ -930,14 +920,6 @@
 		 * as we assume that the client is not interested in
 		 * this case. */
 		return create_ok_response(endp, 200, "DLCX", p->trans);
-	}
-
-	/* Parse the connection id */
-	if (mgcp_parse_ci(&conn_id, ci)) {
-		LOGP(DLMGCP, LOGL_ERROR,
-		     "DLCX: endpoint:%x insufficient parameters, invalid ci (connectionIdentifier)\n",
-		     ENDPOINT_NUMBER(endp));
-		return create_err_response(endp, 400, "DLCX", p->trans);
 	}
 
 	/* Find the connection */
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index f45d6e7..666b8c2 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -365,7 +365,7 @@
 
 	rc = msgb_printf(sdp,
 			 "v=0\r\n"
-			 "o=- %u 23 IN IP4 %s\r\n"
+			 "o=- %s 23 IN IP4 %s\r\n"
 			 "s=-\r\n"
 			 "c=IN IP4 %s\r\n"
 			 "t=0 0\r\n", conn->conn->id, addr, addr);
diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c
index 06420dd..09739c1 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -970,7 +970,7 @@
 	struct mgcp_trunk_config *trunk;
 	struct mgcp_endpoint *endp;
 	struct mgcp_conn_rtp *conn;
-	uint32_t conn_id;
+        const char *conn_id = NULL;
 
 	trunk = find_trunk(g_cfg, atoi(argv[0]));
 	if (!trunk) {
@@ -994,11 +994,11 @@
 
 	endp = &trunk->endpoints[endp_no];
 
-	conn_id = strtoul(argv[2], NULL, 10);
+	conn_id = argv[2];
 	conn = mgcp_conn_get_rtp(endp, conn_id);
 	if (!conn) {
-		vty_out(vty, "Conn ID %s/%d is invalid.%s",
-			argv[2], conn_id, VTY_NEWLINE);
+		vty_out(vty, "Conn ID %s is invalid.%s",
+			conn_id, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
 
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 99491e6..451174d 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -605,7 +605,7 @@
 		if (last_endpoint != -1) {
 			endp = &cfg->trunk.endpoints[last_endpoint];
 
-			conn = mgcp_conn_get_rtp(endp, 1);
+			conn = mgcp_conn_get_rtp(endp, "1");
 			if (conn) {
 				OSMO_ASSERT(conn);
 
@@ -1033,9 +1033,9 @@
 	endp.tcfg = &trunk;
 
 	INIT_LLIST_HEAD(&endp.conns);
-	mgcp_conn_alloc(NULL, &endp, 4711, MGCP_CONN_TYPE_RTP,
+	mgcp_conn_alloc(NULL, &endp, "4711", MGCP_CONN_TYPE_RTP,
 			"test-connection");
-	conn = mgcp_conn_get_rtp(&endp, 4711);
+	conn = mgcp_conn_get_rtp(&endp, "4711");
 	OSMO_ASSERT(conn);
 
 	rtp = &conn->end;
@@ -1111,7 +1111,7 @@
 
 	OSMO_ASSERT(last_endpoint == 1);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 4711);
+	conn = mgcp_conn_get_rtp(endp, "4711");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 18);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == 97);
@@ -1125,7 +1125,7 @@
 
 	OSMO_ASSERT(last_endpoint == 2);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 90210);
+	conn = mgcp_conn_get_rtp(endp, "90210");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 18);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == 97);
@@ -1139,7 +1139,7 @@
 
 	OSMO_ASSERT(last_endpoint == 3);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 815);
+	conn = mgcp_conn_get_rtp(endp, "0815");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == -1);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == -1);
@@ -1153,7 +1153,7 @@
 
 	OSMO_ASSERT(last_endpoint == 4);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 32168);
+	conn = mgcp_conn_get_rtp(endp, "32168");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 18);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == -1);
@@ -1170,7 +1170,7 @@
 
 	OSMO_ASSERT(last_endpoint == 5);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 3);
+	conn = mgcp_conn_get_rtp(endp, "3");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 3);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == -1);
@@ -1182,7 +1182,7 @@
 	msgb_free(resp);
 	OSMO_ASSERT(last_endpoint == 5);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 3);
+	conn = mgcp_conn_get_rtp(endp, "3");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 3);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == -1);
@@ -1199,7 +1199,7 @@
 	talloc_free(endp->last_response);
 	talloc_free(endp->last_trans);
 	endp->last_response = endp->last_trans = NULL;
-	conn = mgcp_conn_get_rtp(endp, 3);
+	conn = mgcp_conn_get_rtp(endp, "3");
 	OSMO_ASSERT(!conn);
 
 	last_endpoint = -1;
@@ -1211,7 +1211,7 @@
 
 	OSMO_ASSERT(last_endpoint == 5);
 	endp = &cfg->trunk.endpoints[last_endpoint];
-	conn = mgcp_conn_get_rtp(endp, 3);
+	conn = mgcp_conn_get_rtp(endp, "3");
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec.payload_type == 255);
 	OSMO_ASSERT(conn->end.alt_codec.payload_type == 0);
@@ -1233,9 +1233,9 @@
 
 	endp = &cfg->trunk.endpoints[1];
 
-	mgcp_conn_alloc(NULL, endp, 4711, MGCP_CONN_TYPE_RTP,
+	mgcp_conn_alloc(NULL, endp, "4711", MGCP_CONN_TYPE_RTP,
 			"test-connection");
-	conn = mgcp_conn_get_rtp(endp, 4711);
+	conn = mgcp_conn_get_rtp(endp, "4711");
 	OSMO_ASSERT(conn);
 
 	OSMO_ASSERT(conn->state.stats_initialized == 0);
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: newpatchset
Gerrit-Change-Id: I0531a1b670d00cec50078423a2868207135b2436
Gerrit-PatchSet: 3
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>



More information about the gerrit-log mailing list