<p>Patch set 3:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +1</span></p><p><a href="https://gerrit.osmocom.org/9402">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/9402/3/src/gprs/gb_proxy_ctrl.c">File src/gprs/gb_proxy_ctrl.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/9402/3/src/gprs/gb_proxy_ctrl.c@47">Patch Set #3, Line 47:</a> <code style="font-family:monospace,monospace">             cmd->reply = talloc_asprintf_append(cmd->reply,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This might be taking it a bit too far, but I think it makes sense to have the "convert a NSVC to a CTRL interface string representation" inside libosmogb.  This way, we can ensure that the same syntax is used when we want to dump the NSVC state of the PCU or the SGSN in [other] patches. What do you think?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/9402/3/src/gprs/gb_proxy_ctrl.c@61">Patch Set #3, Line 61:</a> <code style="font-family:monospace,monospace">CTRL_CMD_DEFINE_RO(ns_status, "ns-state");</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">as we're printing NSVC state, I think it might also make sense to call the command "nsvc-state".  This way we make it clear it's about the NSVC and not the NSE, e.g.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/9402">change 9402</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/9402"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I82c74fd0bfcb9ba4ec3619d9fdaa0cae201b3177 </div>
<div style="display:none"> Gerrit-Change-Number: 9402 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 06 Jun 2018 19:30:44 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>