mgcp_protocol: get rid of policy_cb and change_cb
The two callback functions policy_cb and change_cb are essentially dead
code. They also make the code more difficult to read and understand.
Lets remove them.
Change-Id: I19f67db1c56473f47338b56114f6bbae8981d067
diff --git a/include/osmocom/mgcp/mgcp.h b/include/osmocom/mgcp/mgcp.h
index 29963cc..b3f2eb5 100644
--- a/include/osmocom/mgcp/mgcp.h
+++ b/include/osmocom/mgcp/mgcp.h
@@ -62,8 +62,6 @@
#define MGCP_POLICY_REJECT 5
#define MGCP_POLICY_DEFER 6
-typedef int (*mgcp_change)(struct mgcp_endpoint *endp, int state);
-typedef int (*mgcp_policy)(struct mgcp_endpoint *endp, int state, const char *transaction_id);
typedef int (*mgcp_reset)(struct mgcp_trunk *cfg);
typedef int (*mgcp_rqnt)(struct mgcp_endpoint *endp, char tone);
@@ -147,8 +145,6 @@
int force_ptime;
- mgcp_change change_cb;
- mgcp_policy policy_cb;
mgcp_reset reset_cb;
mgcp_rqnt rqnt_cb;
void *data;
diff --git a/include/osmocom/mgcp/mgcp_protocol.h b/include/osmocom/mgcp/mgcp_protocol.h
index 7ab283d..df134ab 100644
--- a/include/osmocom/mgcp/mgcp_protocol.h
+++ b/include/osmocom/mgcp/mgcp_protocol.h
@@ -16,6 +16,8 @@
int pkt_period_max; /* time in ms */
};
+char *mgcp_debug_get_last_endpoint_name(void);
+
char *get_lco_identifier(const char *options);
int check_local_cx_options(void *ctx, const char *options);
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index e69c00f..b7368b2 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -46,6 +46,16 @@
#include <osmocom/mgcp/mgcp_codec.h>
#include <osmocom/mgcp/mgcp_conn.h>
+/* Contains the last successfully resolved endpoint name. This variable is used
+ * for the unit-tests to verify that the endpoint was correctly resolved. */
+static char debug_last_endpoint_name[MGCP_ENDPOINT_MAXLEN];
+
+/* Called from unit-tests only */
+char *mgcp_debug_get_last_endpoint_name(void)
+{
+ return debug_last_endpoint_name;
+}
+
/* A combination of LOGPENDP and LOGPTRUNK that automatically falls back to
* LOGPTRUNK when the endp parameter is NULL */
#define LOGPEPTR(endp, trunk, cat, level, fmt, args...) \
@@ -328,6 +338,8 @@
struct msgb *resp = NULL;
char *data;
+ debug_last_endpoint_name[0] = '\0';
+
/* Count all messages, even incorect ones */
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_GENERAL_RX_MSGS_TOTAL));
@@ -395,6 +407,7 @@
return create_err_response(NULL, -rq.mgcp_cause, rq.name, pdata.trans);
}
} else {
+ osmo_strlcpy(debug_last_endpoint_name, rq.endp->name, sizeof(debug_last_endpoint_name));
rq.trunk = rq.endp->trunk;
rq.mgcp_cause = 0;
@@ -1050,32 +1063,8 @@
goto error2;
}
- /* policy CB */
- if (pdata->cfg->policy_cb) {
- int rc;
- rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_CRCX, pdata->trans);
- switch (rc) {
- case MGCP_POLICY_REJECT:
- LOGPCONN(_conn, DLMGCP, LOGL_NOTICE,
- "CRCX: CRCX rejected by policy\n");
- mgcp_endp_release(endp);
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_CRCX_FAIL_REJECTED_BY_POLICY));
- return create_err_response(endp, 400, "CRCX", pdata->trans);
- break;
- case MGCP_POLICY_DEFER:
- /* stop processing */
- return NULL;
- break;
- case MGCP_POLICY_CONT:
- /* just continue */
- break;
- }
- }
-
LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
"CRCX: Creating connection: port: %u\n", conn->end.local_port);
- if (pdata->cfg->change_cb)
- pdata->cfg->change_cb(endp, MGCP_ENDP_CRCX);
/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1290,40 +1279,11 @@
goto error3;
}
-
- /* policy CB */
- if (pdata->cfg->policy_cb) {
- int rc;
- rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_MDCX, pdata->trans);
- switch (rc) {
- case MGCP_POLICY_REJECT:
- LOGPCONN(conn->conn, DLMGCP, LOGL_NOTICE,
- "MDCX: rejected by policy\n");
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_FAIL_REJECTED_BY_POLICY));
- if (silent)
- goto out_silent;
- return create_err_response(endp, 400, "MDCX", pdata->trans);
- break;
- case MGCP_POLICY_DEFER:
- /* stop processing */
- LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
- "MDCX: deferred by policy\n");
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_MDCX_DEFERRED_BY_POLICY));
- return NULL;
- break;
- case MGCP_POLICY_CONT:
- /* just continue */
- break;
- }
- }
-
mgcp_rtp_end_config(endp, 1, &conn->end);
/* modify */
LOGPCONN(conn->conn, DLMGCP, LOGL_DEBUG,
"MDCX: modified conn:%s\n", mgcp_conn_dump(conn->conn));
- if (pdata->cfg->change_cb)
- pdata->cfg->change_cb(endp, MGCP_ENDP_MDCX);
/* Send dummy packet, see also comments in mgcp_keepalive_timer_cb() */
OSMO_ASSERT(trunk->keepalive_interval >= MGCP_KEEPALIVE_ONCE);
@@ -1431,29 +1391,6 @@
}
}
- /* policy CB */
- if (pdata->cfg->policy_cb) {
- int rc;
- rc = pdata->cfg->policy_cb(endp, MGCP_ENDP_DLCX, pdata->trans);
- switch (rc) {
- case MGCP_POLICY_REJECT:
- LOGPENDP(endp, DLMGCP, LOGL_NOTICE, "DLCX: rejected by policy\n");
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_FAIL_REJECTED_BY_POLICY));
- if (silent)
- goto out_silent;
- return create_err_response(endp, 400, "DLCX", pdata->trans);
- break;
- case MGCP_POLICY_DEFER:
- /* stop processing */
- rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_DEFERRED_BY_POLICY));
- return NULL;
- break;
- case MGCP_POLICY_CONT:
- /* just continue */
- break;
- }
- }
-
/* Handle wildcarded DLCX that refers to the whole trunk. This means
* that we walk over all endpoints on the trunk in order to drop all
* connections on the trunk. (see also RFC3435 Annex F.7) */
@@ -1515,9 +1452,6 @@
LOGPENDP(endp, DLMGCP, LOGL_DEBUG, "DLCX: endpoint released\n");
}
- if (pdata->cfg->change_cb)
- pdata->cfg->change_cb(endp, MGCP_ENDP_DLCX);
-
rate_ctr_inc(rate_ctr_group_get_ctr(rate_ctrs, MGCP_DLCX_SUCCESS));
if (silent)
goto out_silent;
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 26fcc2a..1c0d3cc 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -614,28 +614,6 @@
return msg;
}
-static char last_endpoint[MGCP_ENDPOINT_MAXLEN];
-
-static int mgcp_test_policy_cb(struct mgcp_endpoint *endp,
- int state, const char *transaction_id)
-{
- unsigned int i;
- struct mgcp_trunk *trunk;
-
- fprintf(stderr, "Policy CB got state %d on endpoint %s\n",
- state, endp->name);
-
- trunk = endp->trunk;
- last_endpoint[0] = '\0';
- for (i = 0; i < trunk->number_endpoints; i++) {
- if (strcmp(endp->name, trunk->endpoints[i]->name) == 0)
- osmo_strlcpy(last_endpoint, trunk->endpoints[i]->name,
- sizeof(last_endpoint));
- }
-
- return MGCP_POLICY_CONT;
-}
-
static int dummy_packets = 0;
/* override and forward */
ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
@@ -792,13 +770,13 @@
struct mgcp_conn_rtp *conn = NULL;
char last_conn_id[256];
int rc;
+ char *last_endpoint = mgcp_debug_get_last_endpoint_name();
cfg = mgcp_config_alloc();
trunk = mgcp_trunk_by_num(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
trunk->v.vty_number_endpoints = 64;
mgcp_trunk_equip(trunk);
- cfg->policy_cb = mgcp_test_policy_cb;
memset(last_conn_id, 0, sizeof(last_conn_id));
@@ -810,7 +788,6 @@
printf("\n================================================\n");
printf("Testing %s\n", t->name);
- last_endpoint[0] = '\0';
dummy_packets = 0;
osmo_talloc_replace_string(cfg, &trunk->audio_fmtp_extra,
@@ -1411,6 +1388,7 @@
struct in_addr addr;
struct mgcp_conn_rtp *conn = NULL;
char conn_id[256];
+ char *last_endpoint = mgcp_debug_get_last_endpoint_name();
printf("Testing multiple payload types\n");
@@ -1418,7 +1396,6 @@
trunk = mgcp_trunk_by_num(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
trunk->v.vty_number_endpoints = 64;
mgcp_trunk_equip(trunk);
- cfg->policy_cb = mgcp_test_policy_cb;
/* Allocate endpoint 1@mgw with two codecs */
last_endpoint[0] = '\0';
@@ -1620,8 +1597,6 @@
trunk->audio_send_name = 0;
mgcp_trunk_equip(trunk);
- cfg->policy_cb = mgcp_test_policy_cb;
-
inp = create_msg(CRCX, NULL);
msg = mgcp_handle_message(cfg, inp);