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@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 <[email protected]>
