<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/11519">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11519/1/include/osmocom/mgcp/mgcp.h">File include/osmocom/mgcp/mgcp.h:</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/11519/1/include/osmocom/mgcp/mgcp.h@197">Patch Set #1, Line 197:</a> <code style="font-family:monospace,monospace">       /* Rate counter group which contains stats for processed CDCX commands. */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">CRCX</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_conn.c">File src/libosmo-mgcp/mgcp_conn.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/11519/1/src/libosmo-mgcp/mgcp_conn.c@41">Patch Set #1, Line 41:</a> <code style="font-family:monospace,monospace">  [RTP_OCTETS_TX_CTR] = {"rtp:octets_tx", "Outbound rtp octets."},</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This looks unrelated. Different patch?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_conn.c@248">Patch Set #1, Line 248:</a> <code style="font-family:monospace,monospace">    OSMO_ASSERT(conn_stats->desc->num_ctr + 1 == all_stats->desc->num_ctr);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you explain this verification?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c">File src/libosmo-mgcp/mgcp_protocol.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/11519/1/src/libosmo-mgcp/mgcp_protocol.c@108">Patch Set #1, Line 108:</a> <code style="font-family:monospace,monospace">static const struct rate_ctr_desc all_rtp_conn_rate_ctr_desc[] = {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Let's better use a macro instead of manually keeping them in sync. That's going to break eventually.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Another possibility: Make an init function which creates this dynamically based on the other one and then appends RTP_NUM_CONNECTIONS.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11519/1/src/libosmo-mgcp/mgcp_protocol.c@1659">Patch Set #1, Line 1659:</a> <code style="font-family:monospace,monospace">      alloc_rate_counters(tcfg, tcfg->cfg);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">why removing mgcp here and other places?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11519">change 11519</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/11519"/><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-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I80d36181600901ae2e0f321dc02b5d54ddc94139 </div>
<div style="display:none"> Gerrit-Change-Number: 11519 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <ssperling@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 30 Oct 2018 14:38:14 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>