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/.
Harald Welte gerrit-no-reply at lists.osmocom.orgHello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/5608
to look at the new patch set (#2).
centralize handling of common errors like "endpoint not found"
Previously we
* did not distinguish between the cause of errors in mgcp_header_parse
* common errors were not handled in mgcp_handle_message() but in
individual per-verb functions
Let's centralize the handling of generating error responses and remove
that burden from the individual per-verb handler functions.
Change-Id: I463b27306e10ae3b021583ed102977e7299e5e66
---
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
M tests/mgcp/mgcp_test.c
3 files changed, 12 insertions(+), 30 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/08/5608/2
diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index d6174df..7f05a44 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -230,21 +230,21 @@
if (!pdata->endp) {
LOGP(DLMGCP, LOGL_ERROR,
"Unable to find Endpoint `%s'\n", elem);
- return -1;
+ return -500;
}
break;
case 2:
if (strcmp("MGCP", elem)) {
LOGP(DLMGCP, LOGL_ERROR,
"MGCP header parsing error\n");
- return -1;
+ return -510;
}
break;
case 3:
if (strcmp("1.0", elem)) {
LOGP(DLMGCP, LOGL_ERROR, "MGCP version `%s' "
"not supported\n", elem);
- return -1;
+ return -528;
}
break;
}
@@ -255,7 +255,7 @@
LOGP(DLMGCP, LOGL_ERROR, "MGCP status line too short.\n");
pdata->trans = "000000";
pdata->endp = NULL;
- return -1;
+ return -510;
}
return 0;
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index f87f341..71c0fde 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -287,6 +287,12 @@
return do_retransmission(pdata.endp);
}
+ /* check for general parser failure */
+ if (pdata.found < 0) {
+ LOGP(DLMGCP, LOGL_NOTICE, "%s: failed to find the endpoint\n", msg->l2h);
+ return create_err_response(NULL, -pdata.found, (const char *) msg->l2h, pdata.trans);
+ }
+
for (i = 0; i < ARRAY_SIZE(mgcp_requests); ++i) {
if (strncmp
(mgcp_requests[i].name, (const char *)&msg->l2h[0],
@@ -308,13 +314,7 @@
static struct msgb *handle_audit_endpoint(struct mgcp_parse_data *p)
{
LOGP(DLMGCP, LOGL_NOTICE, "AUEP: auditing endpoint ...\n");
-
- if (p->found != 0) {
- LOGP(DLMGCP, LOGL_ERROR,
- "AUEP: failed to find the endpoint.\n");
- return create_err_response(NULL, 500, "AUEP", p->trans);
- } else
- return create_ok_response(p->endp, 200, "AUEP", p->trans);
+ return create_ok_response(p->endp, 200, "AUEP", p->trans);
}
/* Try to find a free port by attempting to bind on it. Also handle the
@@ -451,9 +451,6 @@
char conn_name[512];
LOGP(DLMGCP, LOGL_NOTICE, "CRCX: creating new connection ...\n");
-
- if (p->found != 0)
- return create_err_response(NULL, 510, "CRCX", p->trans);
/* parse CallID C: and LocalParameters L: */
for_each_line(line, p->save) {
@@ -675,9 +672,6 @@
LOGP(DLMGCP, LOGL_NOTICE, "MDCX: modifying existing connection ...\n");
- if (p->found != 0)
- return create_err_response(NULL, 510, "MDCX", p->trans);
-
if (llist_count(&endp->conns) <= 0) {
LOGP(DLMGCP, LOGL_ERROR,
"MDCX: endpoint:0x%x endpoint is not holding a connection.\n",
@@ -824,9 +818,6 @@
const char *conn_id = NULL;
struct mgcp_conn_rtp *conn = NULL;
- if (p->found != 0)
- return create_err_response(NULL, error_code, "DLCX", p->trans);
-
LOGP(DLMGCP, LOGL_NOTICE,
"DLCX: endpoint:0x%x deleting connection ...\n",
ENDPOINT_NUMBER(endp));
@@ -958,12 +949,6 @@
LOGP(DLMGCP, LOGL_NOTICE, "RSIP: resetting all endpoints ...\n");
- if (p->found != 0) {
- LOGP(DLMGCP, LOGL_ERROR,
- "RSIP: failed to find the endpoint.\n");
- return NULL;
- }
-
if (p->cfg->reset_cb)
p->cfg->reset_cb(p->endp->tcfg);
return NULL;
@@ -988,9 +973,6 @@
char tone = CHAR_MAX;
LOGP(DLMGCP, LOGL_NOTICE, "RQNT: processing request for notification ...\n");
-
- if (p->found != 0)
- return create_err_response(NULL, 400, "RQNT", p->trans);
for_each_line(line, p->save) {
switch (line[0]) {
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 46fc1c1..46fd69b 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -76,7 +76,7 @@
#define SHORT_RET "510 000000 FAIL\r\n"
#define MDCX_WRONG_EP "MDCX 18983213 ds/e1-3/1 at 172.16.6.66 MGCP 1.0\r\n"
-#define MDCX_ERR_RET "510 18983213 FAIL\r\n"
+#define MDCX_ERR_RET "500 18983213 FAIL\r\n"
#define MDCX_UNALLOCATED "MDCX 18983214 ds/e1-1/2 at 172.16.6.66 MGCP 1.0\r\n"
#define MDCX_RET "400 18983214 FAIL\r\n"
--
To view, visit https://gerrit.osmocom.org/5608
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I463b27306e10ae3b021583ed102977e7299e5e66
Gerrit-PatchSet: 2
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder