<p style="white-space: pre-wrap; word-wrap: break-word;">Apparently gerrit does not work the way it should. Apparantly pending comments are not sent, even after pushing the changes. Please do not merge this or comment any further until I get back to you again.</p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590">View Change</a></p><p>16 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_endp.c">File src/libosmo-mgcp/mgcp_endp.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_endp.c@a100">Patch Set #4, Line 100:</a> <code style="font-family:monospace,monospace"> if (strlen(epname) <= prefix_len)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">unrelated?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_osmux.c">File src/libosmo-mgcp/mgcp_osmux.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_osmux.c@207">Patch Set #4, Line 207:</a> <code style="font-family:monospace,monospace"> for (i=0; i<trunk->number_endpoints; i++) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">since you are changing the line you could fix the missing spacing too.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_trunk.c">File src/libosmo-mgcp/mgcp_trunk.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_trunk.c@34">Patch Set #4, Line 34:</a> <code style="font-family:monospace,monospace">int mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">alloc returning something than a pointer is confusing. Better return the pointer or NULL.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_vty.c">File src/libosmo-mgcp/mgcp_vty.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/4/src/libosmo-mgcp/mgcp_vty.c@852">Patch Set #4, Line 852:</a> <code style="font-family:monospace,monospace"> * parameters, so wie restrict the access to trunks with id numbers</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">typo we</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c">File src/libosmo-mgcp/mgcp_vty.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@557">Patch Set #10, Line 557:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@572">Patch Set #10, Line 572:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@582">Patch Set #10, Line 582:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@620">Patch Set #10, Line 620:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@631">Patch Set #10, Line 631:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@642">Patch Set #10, Line 642:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">OSMO_ASSERT(trunk)?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@653">Patch Set #10, Line 653:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">...</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@880">Patch Set #10, Line 880:</a> <code style="font-family:monospace,monospace"> continue;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">so, where/how is the virtual trunk written to config?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The config for the virtual trunk is written in config_write_mgcp().</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/10/src/libosmo-mgcp/mgcp_vty.c@891">Patch Set #10, Line 891:</a> <code style="font-family:monospace,monospace"> vty_out(vty, " rtp keep-alive %d%s",</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">There are vty commands with the same name, but on different nodes. See mgcp_vty_init(). This is very confusing, I got confused too. I have removed those fixme notes now.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c">File src/libosmo-mgcp/mgcp_vty.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@751">Patch Set #11, Line 751:</a> <code style="font-family:monospace,monospace"> OSMO_ASSERT(trunk);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">thanks for applying here...</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@760">Patch Set #11, Line 760:</a> <code style="font-family:monospace,monospace"> struct mgcp_trunk *trunk = mgcp_trunk_by_num(g_cfg, MGCP_VIRT_TRUNK_ID);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">...and there are still many more of these below […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Thanks for catching this. I overlooked the remaining ones, probably my search string was too distinctive.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590/11/src/libosmo-mgcp/mgcp_vty.c@882">Patch Set #11, Line 882:</a> <code style="font-family:monospace,monospace"> continue;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">please still explain, as in previous comment: […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I would love to eliminate config_write_mgcp() completely, but I think this changes the syntax of the configuration file too much. Also I think the time is not right for that yet. When the E1 support is done we have two different trunk sets, and then we can see much better how we can find a more meaningful configuration scheme. I think it would also be very nice to be able to create multiple virtual trunks. Think of one MGW that handles BSC and MSC on two different virtual trunks.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18590">change 18590</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/+/18590"/><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: I81934fbd211b225ab7920e78510729c8e22607b3 </div>
<div style="display:none"> Gerrit-Change-Number: 18590 </div>
<div style="display:none"> Gerrit-PatchSet: 12 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 18 Jun 2020 09:16:58 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>