<p>fixeria <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22164">View Change</a></p><div style="white-space:pre-wrap">Approvals:
pespin: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">vty: fix writing empty IP address for unconfigured NSVCs<br><br>config_write_bts_gprs() currently writes the remote address of an<br>NSVC even if osmo_sockaddr_str_from_sockaddr() returns non-zero<br>code. Thus saving a configuration with only one configured NSVC<br>to a file would produce the following:<br><br> bts N<br> ...<br> gprs nsvc 0 nsvci 101<br> gprs nsvc 0 local udp port 23023<br> gprs nsvc 0 remote ip 127.0.0.1<br> gprs nsvc 0 remote udp port 23000<br> gprs nsvc 1 nsvci 0<br> gprs nsvc 1 local udp port 0<br> gprs nsvc 1 remote ip<br><br>and next time osmo-bsc would refuse to start due to:<br><br> Error occurred during reading the below line:<br> gprs nsvc 1 remote ip<br><br>The related condition consists of the following two parts:<br><br> - checking if osmo_sockaddr_str_from_sockaddr() != 0;<br> - checking if 'remote.af != AF_UNSPEC'.<br><br>The first one is wrong, because osmo_sockaddr_str_from_sockaddr(),<br>like many other functions, returns 0 on success. Let's fix this.<br><br>After the fix, the second part does not seem to make sense, because<br>remote.af would remain AF_UNSPEC (0) if the function call succeeds.<br><br>Printing the remote port alone does not make sense, let's avoid<br>printing it if the address cannot be parsed into a string.<br><br>Change-Id: I5d6cbde4f605c8184db4ade87de5644a849c05db<br>Fixes: I621360cab1e12c22248e33d62a9929995053ce04<br>---<br>M src/osmo-bsc/bsc_vty.c<br>1 file changed, 9 insertions(+), 13 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c</span><br><span>index 91acb03..753acf7 100644</span><br><span>--- a/src/osmo-bsc/bsc_vty.c</span><br><span>+++ b/src/osmo-bsc/bsc_vty.c</span><br><span>@@ -772,8 +772,7 @@</span><br><span> bts_sm->gprs.nse.timer[i], VTY_NEWLINE);</span><br><span> for (i = 0; i < ARRAY_SIZE(bts_sm->gprs.nsvc); i++) {</span><br><span> const struct gsm_gprs_nsvc *nsvc = &bts_sm->gprs.nsvc[i];</span><br><span style="color: hsl(0, 100%, 40%);">- struct osmo_sockaddr_str remote = {};</span><br><span style="color: hsl(0, 100%, 40%);">- uint16_t port;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct osmo_sockaddr_str remote;</span><br><span> </span><br><span> vty_out(vty, " gprs nsvc %u nsvci %u%s", i,</span><br><span> nsvc->nsvci, VTY_NEWLINE);</span><br><span>@@ -781,18 +780,15 @@</span><br><span> vty_out(vty, " gprs nsvc %u local udp port %u%s", i,</span><br><span> nsvc->local_port, VTY_NEWLINE);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (osmo_sockaddr_str_from_sockaddr(&remote, &nsvc->remote.u.sas) ||</span><br><span style="color: hsl(0, 100%, 40%);">- remote.af != AF_UNSPEC) {</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " gprs nsvc %u remote ip %s%s", i,</span><br><span style="color: hsl(0, 100%, 40%);">- remote.ip, VTY_NEWLINE);</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Most likely, the remote address is not configured (AF_UNSPEC).</span><br><span style="color: hsl(120, 100%, 40%);">+ * Printing the port alone makes no sense, so let's just skip both. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (osmo_sockaddr_str_from_sockaddr(&remote, &nsvc->remote.u.sas) != 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ continue;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Can't use remote.port because it's only valid when family != AF_UNSPEC, but the</span><br><span style="color: hsl(0, 100%, 40%);">- * port can be even configured when the IP isn't */</span><br><span style="color: hsl(0, 100%, 40%);">- port = osmo_htons(nsvc->remote.u.sin.sin_port);</span><br><span style="color: hsl(0, 100%, 40%);">- if (port)</span><br><span style="color: hsl(0, 100%, 40%);">- vty_out(vty, " gprs nsvc %u remote udp port %u%s", i,</span><br><span style="color: hsl(0, 100%, 40%);">- port, VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, " gprs nsvc %u remote ip %s%s",</span><br><span style="color: hsl(120, 100%, 40%);">+ i, remote.ip, VTY_NEWLINE);</span><br><span style="color: hsl(120, 100%, 40%);">+ vty_out(vty, " gprs nsvc %u remote udp port %u%s",</span><br><span style="color: hsl(120, 100%, 40%);">+ i, remote.port, VTY_NEWLINE);</span><br><span> }</span><br><span> </span><br><span> /* EGPRS specific parameters */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/22164">change 22164</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-bsc/+/22164"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I5d6cbde4f605c8184db4ade87de5644a849c05db </div>
<div style="display:none"> Gerrit-Change-Number: 22164 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>