<p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp">File src/gprs_bssgp_pcu.cpp:</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-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@928">Patch Set #1, Line 928:</a> <code style="font-family:monospace,monospace">                   int valid)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">it's a bitmask right? rather use unsigned then, and probably better of fixed length.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@931">Patch Set #1, Line 931:</a> <code style="font-family:monospace,monospace">       int binds = 0, nsvcs = 0;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">same, bitmask.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@938">Patch Set #1, Line 938:</a> <code style="font-family:monospace,monospace">    for (i=0; i < PCU_IF_NUM_NSVC; i++) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@943">Patch Set #1, Line 943:</a> <code style="font-family:monospace,monospace">              if (rc < 0 && rc != -EBUSY) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I?m not really liking this no-op silent behavior when -EBUSY is returned. IMHO this code should be the one checking if the binding already exists and attempting to bind or do nothing. It shouldn't be done transparently by the library layer.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@966">Patch Set #1, Line 966:</a> <code style="font-family:monospace,monospace">       for (i=0; i < PCU_IF_NUM_NSVC; i++) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@980">Patch Set #1, Line 980:</a> <code style="font-family:monospace,monospace">                              nsvcs |= 1 << i;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">so no use for this bitmask, simply store a bool</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565/1/src/gprs_bssgp_pcu.cpp@1068">Patch Set #1, Line 1068:</a> <code style="font-family:monospace,monospace">            /* we found all our valid nsvcs and might removed all other nsvcs */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">might have removed?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/20565">change 20565</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-pcu/+/20565"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I589ebaa2a2b7de55b7e4e975d8fd6412dd5f214b </div>
<div style="display:none"> Gerrit-Change-Number: 20565 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 12 Oct 2020 10:13:44 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>