<p>pespin has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/15580">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">mgcp_test: Correctly release all endpoints allocated<br><br>Currently in handle_create_con(), mgcp_conn_alloc() is called with NULl<br>ctx. As soon as this ctx is changed to be part of the trunk's endpoint<br>array (tcfg->endpoints), test will segfault because some fds from<br>previous tcfg are still registered after the whole tcfg object was freed<br>with talloc_free() by previous test. That's because<br>mgcp_endpoint_release() must be called on all endpoints to make sure all<br>registered components are correctly unplugged.<br><br>Related: OS#3950<br>Change-Id: I813d52b518ed0bb8db4e42dff83e040b0891fee2<br>---<br>M tests/mgcp/mgcp_test.c<br>1 file changed, 34 insertions(+), 11 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/80/15580/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c</span><br><span>index e5dec2a..c72382e 100644</span><br><span>--- a/tests/mgcp/mgcp_test.c</span><br><span>+++ b/tests/mgcp/mgcp_test.c</span><br><span>@@ -641,6 +641,13 @@</span><br><span>     return real_clock_gettime(clk_id, tp);</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static void mgcp_endpoints_release(struct mgcp_trunk_config *trunk)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+       int i;</span><br><span style="color: hsl(120, 100%, 40%);">+        for (i = 1; i < trunk->number_endpoints; i++)</span><br><span style="color: hsl(120, 100%, 40%);">+           mgcp_endp_release(&trunk->endpoints[i]);</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #define CONN_UNMODIFIED (0x1000)</span><br><span> </span><br><span> static void test_values(void)</span><br><span>@@ -742,6 +749,7 @@</span><br><span> {</span><br><span>     struct mgcp_config *cfg;</span><br><span>     struct mgcp_endpoint *endp;</span><br><span style="color: hsl(120, 100%, 40%);">+   struct mgcp_trunk_config *trunk2;</span><br><span>    int i;</span><br><span>       struct mgcp_conn_rtp *conn = NULL;</span><br><span>   char last_conn_id[256];</span><br><span>@@ -755,7 +763,8 @@</span><br><span> </span><br><span>    memset(last_conn_id, 0, sizeof(last_conn_id));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));</span><br><span style="color: hsl(120, 100%, 40%);">+    trunk2 = mgcp_trunk_alloc(cfg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+    mgcp_endpoints_allocate(trunk2);</span><br><span> </span><br><span>         for (i = 0; i < ARRAY_SIZE(tests); i++) {</span><br><span>                 const struct mgcp_test *t = &tests[i];</span><br><span>@@ -873,12 +882,15 @@</span><br><span>           }</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ mgcp_endpoints_release(trunk2);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span> static void test_retransmission(void)</span><br><span> {</span><br><span>     struct mgcp_config *cfg;</span><br><span style="color: hsl(120, 100%, 40%);">+      struct mgcp_trunk_config *trunk2;</span><br><span>    int i;</span><br><span>       char last_conn_id[256];</span><br><span>      int rc;</span><br><span>@@ -890,7 +902,8 @@</span><br><span> </span><br><span>    memset(last_conn_id, 0, sizeof(last_conn_id));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));</span><br><span style="color: hsl(120, 100%, 40%);">+    trunk2 = mgcp_trunk_alloc(cfg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+    mgcp_endpoints_allocate(trunk2);</span><br><span> </span><br><span>         for (i = 0; i < ARRAY_SIZE(retransmit); i++) {</span><br><span>            const struct mgcp_test *t = &retransmit[i];</span><br><span>@@ -930,6 +943,8 @@</span><br><span>                msgb_free(msg);</span><br><span>      }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ mgcp_endpoints_release(trunk2);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span>@@ -943,6 +958,7 @@</span><br><span> static void test_rqnt_cb(void)</span><br><span> {</span><br><span>      struct mgcp_config *cfg;</span><br><span style="color: hsl(120, 100%, 40%);">+      struct mgcp_trunk_config *trunk2;</span><br><span>    struct msgb *inp, *msg;</span><br><span>      char conn_id[256];</span><br><span> </span><br><span>@@ -952,7 +968,8 @@</span><br><span>         cfg->trunk.vty_number_endpoints = 64;</span><br><span>     mgcp_endpoints_allocate(&cfg->trunk);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-        mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));</span><br><span style="color: hsl(120, 100%, 40%);">+    trunk2 = mgcp_trunk_alloc(cfg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+    mgcp_endpoints_allocate(trunk2);</span><br><span> </span><br><span>         inp = create_msg(CRCX, NULL);</span><br><span>        msg = mgcp_handle_message(cfg, inp);</span><br><span>@@ -981,6 +998,8 @@</span><br><span>   inp = create_msg(DLCX, conn_id);</span><br><span>     msgb_free(mgcp_handle_message(cfg, inp));</span><br><span>    msgb_free(inp);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(trunk2);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span>@@ -1342,12 +1361,12 @@</span><br><span> static void test_multilple_codec(void)</span><br><span> {</span><br><span>  struct mgcp_config *cfg;</span><br><span style="color: hsl(120, 100%, 40%);">+      struct mgcp_trunk_config *trunk2;</span><br><span>    struct mgcp_endpoint *endp;</span><br><span>  struct msgb *inp, *resp;</span><br><span>     struct in_addr addr;</span><br><span>         struct mgcp_conn_rtp *conn = NULL;</span><br><span>   char conn_id[256];</span><br><span style="color: hsl(0, 100%, 40%);">-      int i;</span><br><span> </span><br><span>   printf("Testing multiple payload types\n");</span><br><span> </span><br><span>@@ -1355,7 +1374,9 @@</span><br><span>    cfg->trunk.vty_number_endpoints = 64;</span><br><span>     mgcp_endpoints_allocate(&cfg->trunk);</span><br><span>         cfg->policy_cb = mgcp_test_policy_cb;</span><br><span style="color: hsl(0, 100%, 40%);">-        mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  trunk2 = mgcp_trunk_alloc(cfg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+    mgcp_endpoints_allocate(trunk2);</span><br><span> </span><br><span>         /* Allocate endpoint 1@mgw with two codecs */</span><br><span>        last_endpoint = -1;</span><br><span>@@ -1481,9 +1502,8 @@</span><br><span>  OSMO_ASSERT(conn);</span><br><span>   OSMO_ASSERT(conn->end.codec->payload_type == 0);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      for (i = 1; i < cfg->trunk.number_endpoints; i++)</span><br><span style="color: hsl(0, 100%, 40%);">-         mgcp_endp_release(&cfg->trunk.endpoints[i]);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(120, 100%, 40%);">+   mgcp_endpoints_release(trunk2);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span>@@ -1532,12 +1552,13 @@</span><br><span>         OSMO_ASSERT(conn->state.stats.cycles == UINT16_MAX + 1);</span><br><span>  OSMO_ASSERT(conn->state.stats.max_seq == 0);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     mgcp_endp_release(endp);</span><br><span style="color: hsl(120, 100%, 40%);">+      mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span> static void test_no_name(void)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+     struct mgcp_trunk_config *trunk2;</span><br><span>    struct mgcp_config *cfg;</span><br><span>     struct msgb *inp, *msg;</span><br><span> </span><br><span>@@ -1550,7 +1571,8 @@</span><br><span> </span><br><span>      cfg->policy_cb = mgcp_test_policy_cb;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    mgcp_endpoints_allocate(mgcp_trunk_alloc(cfg, 1));</span><br><span style="color: hsl(120, 100%, 40%);">+    trunk2 = mgcp_trunk_alloc(cfg, 1);</span><br><span style="color: hsl(120, 100%, 40%);">+    mgcp_endpoints_allocate(trunk2);</span><br><span> </span><br><span>         inp = create_msg(CRCX, NULL);</span><br><span>        msg = mgcp_handle_message(cfg, inp);</span><br><span>@@ -1563,7 +1585,8 @@</span><br><span>         msgb_free(inp);</span><br><span>      msgb_free(msg);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     mgcp_endp_release(&cfg->trunk.endpoints[1]);</span><br><span style="color: hsl(120, 100%, 40%);">+   mgcp_endpoints_release(trunk2);</span><br><span style="color: hsl(120, 100%, 40%);">+       mgcp_endpoints_release(&cfg->trunk);</span><br><span>  talloc_free(cfg);</span><br><span> }</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/15580">change 15580</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-mgw/+/15580"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I813d52b518ed0bb8db4e42dff83e040b0891fee2 </div>
<div style="display:none"> Gerrit-Change-Number: 15580 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>