Change in ...osmo-mgw[master]: mgcp_test: Correctly release all endpoints allocated

[laforge]

laforge has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-mgw/+/15580 )

Change subject: mgcp_test: Correctly release all endpoints allocated
......................................................................

mgcp_test: Correctly release all endpoints allocated

Currently in handle_create_con(), mgcp_conn_alloc() is called with NULl
ctx. As soon as this ctx is changed to be part of the trunk's endpoint
array (tcfg->endpoints), test will segfault because some fds from
previous tcfg are still registered after the whole tcfg object was freed
with talloc_free() by previous test. That's because
mgcp_endpoint_release() must be called on all endpoints to make sure all
registered components are correctly unplugged.

Related: OS#3950
Change-Id: I813d52b518ed0bb8db4e42dff83e040b0891fee2
---
M tests/mgcp/mgcp_test.c
1 file changed, 34 insertions(+), 11 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index e5dec2a..c72382e 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -641,6 +641,13 @@
 	return real_clock_gettime(clk_id, tp);
 }
 
+static void mgcp_endpoints_release(struct mgcp_trunk_config *trunk)
+{
+	int i;
+	for (i = 1; i < trunk->number_endpoints; i++)
+		mgcp_endp_release(&trunk->endpoints[i]);
+}
+
 #define CONN_UNMODIFIED (0x1000)
 
 static void test_values(void)
@@ -742,6 +749,7 @@
 {
 	struct mgcp_config *cfg;
 	struct mgcp_endpoint *endp;
+	struct mgcp_trunk_config *trunk2;
 	int i;
 	struct mgcp_conn_rtp *conn = NULL;
 	char last_conn_id[256];
@@ -755,7 +763,8 @@
 
 	memset(last_conn_id, 0, sizeof(last_conn_id));
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	for (i = 0; i < ARRAY_SIZE(tests); i++) {
 		const struct mgcp_test *t = &tests[i];
@@ -873,12 +882,15 @@
 		}
 	}
 
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
 static void test_retransmission(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	int i;
 	char last_conn_id[256];
 	int rc;
@@ -890,7 +902,8 @@
 
 	memset(last_conn_id, 0, sizeof(last_conn_id));
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	for (i = 0; i < ARRAY_SIZE(retransmit); i++) {
 		const struct mgcp_test *t = &retransmit[i];
@@ -930,6 +943,8 @@
 		msgb_free(msg);
 	}
 
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -943,6 +958,7 @@
 static void test_rqnt_cb(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	struct msgb *inp, *msg;
 	char conn_id[256];
 
@@ -952,7 +968,8 @@
 	cfg->trunk.vty_number_endpoints = 64;
 	mgcp_endpoints_allocate(&cfg->trunk);
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	inp = create_msg(CRCX, NULL);
 	msg = mgcp_handle_message(cfg, inp);
@@ -981,6 +998,8 @@
 	inp = create_msg(DLCX, conn_id);
 	msgb_free(mgcp_handle_message(cfg, inp));
 	msgb_free(inp);
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -1342,12 +1361,12 @@
 static void test_multilple_codec(void)
 {
 	struct mgcp_config *cfg;
+	struct mgcp_trunk_config *trunk2;
 	struct mgcp_endpoint *endp;
 	struct msgb *inp, *resp;
 	struct in_addr addr;
 	struct mgcp_conn_rtp *conn = NULL;
 	char conn_id[256];
-	int i;
 
 	printf("Testing multiple payload types\n");
 
@@ -1355,7 +1374,9 @@
 	cfg->trunk.vty_number_endpoints = 64;
 	mgcp_endpoints_allocate(&cfg->trunk);
 	cfg->policy_cb = mgcp_test_policy_cb;
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	/* Allocate endpoint 1 at mgw with two codecs */
 	last_endpoint = -1;
@@ -1481,9 +1502,8 @@
 	OSMO_ASSERT(conn);
 	OSMO_ASSERT(conn->end.codec->payload_type == 0);
 
-	for (i = 1; i < cfg->trunk.number_endpoints; i++)
-		mgcp_endp_release(&cfg->trunk.endpoints[i]);
-
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
@@ -1532,12 +1552,13 @@
 	OSMO_ASSERT(conn->state.stats.cycles == UINT16_MAX + 1);
 	OSMO_ASSERT(conn->state.stats.max_seq == 0);
 
-	mgcp_endp_release(endp);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 
 static void test_no_name(void)
 {
+	struct mgcp_trunk_config *trunk2;
 	struct mgcp_config *cfg;
 	struct msgb *inp, *msg;
 
@@ -1550,7 +1571,8 @@
 
 	cfg->policy_cb = mgcp_test_policy_cb;
 
-	mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));
+	trunk2 = mgcp_trunk_alloc(cfg, 1);
+	mgcp_endpoints_allocate(trunk2);
 
 	inp = create_msg(CRCX, NULL);
 	msg = mgcp_handle_message(cfg, inp);
@@ -1563,7 +1585,8 @@
 	msgb_free(inp);
 	msgb_free(msg);
 
-	mgcp_endp_release(&cfg->trunk.endpoints[1]);
+	mgcp_endpoints_release(trunk2);
+	mgcp_endpoints_release(&cfg->trunk);
 	talloc_free(cfg);
 }
 

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/15580
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I813d52b518ed0bb8db4e42dff83e040b0891fee2
Gerrit-Change-Number: 15580
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190920/40e4df5e/attachment.htm>