Change in osmo-bsc[master]: vty: fix writing empty IP address for unconfigured NSVC

[fixeria]

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/22164 )


Change subject: vty: fix writing empty IP address for unconfigured NSVC
......................................................................

vty: fix writing empty IP address for unconfigured NSVC

config_write_bts_gprs() currently writes the remote address of an
NSVC even if osmo_sockaddr_str_from_sockaddr() returns non-zero
code.  Thus saving a configuration with only one configured NSVC
to a file would produce the following:

 bts N
  ...
  gprs nsvc 0 nsvci 101
  gprs nsvc 0 local udp port 23023
  gprs nsvc 0 remote ip 127.0.0.1
  gprs nsvc 0 remote udp port 23000
  gprs nsvc 1 nsvci 0
  gprs nsvc 1 local udp port 0
  gprs nsvc 1 remote ip

and next time osmo-bsc would refuse to start due to:

 Error occurred during reading the below line:
  gprs nsvc 1 remote ip

The related condition consists of the following two parts:

  - checking if osmo_sockaddr_str_from_sockaddr() != 0;
  - checking if 'remote.af != AF_UNSPEC'.

The first one is wrong, because osmo_sockaddr_str_from_sockaddr(),
like many other functions, returns 0 on success.  Let's fix this.

After the fix, the second part does not seem to make sense, because
remote.af would remain AF_UNSPEC (0) if the function call succeeds.

Printing the remote port alone does not make sense, let's avoid
printing it if the address cannot be parsed into a string.

Change-Id: I5d6cbde4f605c8184db4ade87de5644a849c05db
Fixes: I621360cab1e12c22248e33d62a9929995053ce04
---
M src/osmo-bsc/bsc_vty.c
1 file changed, 9 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/64/22164/1

diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index 91acb03..753acf7 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -772,8 +772,7 @@
 			bts_sm->gprs.nse.timer[i], VTY_NEWLINE);
 	for (i = 0; i < ARRAY_SIZE(bts_sm->gprs.nsvc); i++) {
 		const struct gsm_gprs_nsvc *nsvc = &bts_sm->gprs.nsvc[i];
-		struct osmo_sockaddr_str remote = {};
-		uint16_t port;
+		struct osmo_sockaddr_str remote;
 
 		vty_out(vty, "  gprs nsvc %u nsvci %u%s", i,
 			nsvc->nsvci, VTY_NEWLINE);
@@ -781,18 +780,15 @@
 		vty_out(vty, "  gprs nsvc %u local udp port %u%s", i,
 			nsvc->local_port, VTY_NEWLINE);
 
-		if (osmo_sockaddr_str_from_sockaddr(&remote, &nsvc->remote.u.sas) ||
-				remote.af != AF_UNSPEC) {
-			vty_out(vty, "  gprs nsvc %u remote ip %s%s", i,
-				remote.ip, VTY_NEWLINE);
-		}
+		/* Most likely, the remote address is not configured (AF_UNSPEC).
+		 * Printing the port alone makes no sense, so let's just skip both. */
+		if (osmo_sockaddr_str_from_sockaddr(&remote, &nsvc->remote.u.sas) != 0)
+			continue;
 
-		/* Can't use remote.port because it's only valid when family != AF_UNSPEC, but the
-		 * port can be even configured when the IP isn't */
-		port = osmo_htons(nsvc->remote.u.sin.sin_port);
-		if (port)
-			vty_out(vty, "  gprs nsvc %u remote udp port %u%s", i,
-				port, VTY_NEWLINE);
+		vty_out(vty, "  gprs nsvc %u remote ip %s%s",
+			i, remote.ip, VTY_NEWLINE);
+		vty_out(vty, "  gprs nsvc %u remote udp port %u%s",
+			i, remote.port, VTY_NEWLINE);
 	}
 
 	/* EGPRS specific parameters */

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/22164
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I5d6cbde4f605c8184db4ade87de5644a849c05db
Gerrit-Change-Number: 22164
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210113/b9ebfecf/attachment.htm>