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.orgHarald Welte has submitted this change and it was merged. Change subject: centralize handling of common errors like "endpoint not found" ...................................................................... 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(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified 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: merged Gerrit-Change-Id: I463b27306e10ae3b021583ed102977e7299e5e66 Gerrit-PatchSet: 2 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