Change in osmo-mgw[master]: mgcp_trunk: use unsigned int instead of int as trunk_nr

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

dexter gerrit-no-reply at lists.osmocom.org
Wed Jul 21 09:12:40 UTC 2021


dexter has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/24963 )

Change subject: mgcp_trunk: use unsigned int instead of int as trunk_nr
......................................................................

mgcp_trunk: use unsigned int instead of int as trunk_nr

the trunk_nr is in struct mgcp_trunk. The trunk number can not be
negative and there is no magic value that makes use of the fact that it
could be negative. Lets use unsigned int to make this less irretating.

Change-Id: I5d0e1d76adb8c92d84331a0aca2496908e41d621
Related: SYS#5535
---
M include/osmocom/mgcp/mgcp_trunk.h
M src/libosmo-mgcp/mgcp_endp.c
M src/libosmo-mgcp/mgcp_ratectr.c
M src/libosmo-mgcp/mgcp_trunk.c
M src/libosmo-mgcp/mgcp_vty.c
M src/osmo-mgw/mgw_main.c
M tests/mgcp/mgcp_test.c
7 files changed, 63 insertions(+), 52 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, but someone else must approve
  daniel: Looks good to me, approved



diff --git a/include/osmocom/mgcp/mgcp_trunk.h b/include/osmocom/mgcp/mgcp_trunk.h
index 326b16a..048ac5b 100644
--- a/include/osmocom/mgcp/mgcp_trunk.h
+++ b/include/osmocom/mgcp/mgcp_trunk.h
@@ -22,7 +22,7 @@
 
 	struct mgcp_config *cfg;
 
-	int trunk_nr;
+	unsigned int trunk_nr;
 	enum mgcp_trunk_type trunk_type;
 
 	char *audio_fmtp_extra;
@@ -72,11 +72,11 @@
 	};
 };
 
-struct mgcp_trunk *mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr);
+struct mgcp_trunk *mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, unsigned int nr);
 int mgcp_trunk_equip(struct mgcp_trunk *trunk);
-struct mgcp_trunk *mgcp_trunk_by_num(const struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr);
+struct mgcp_trunk *mgcp_trunk_by_num(const struct mgcp_config *cfg, enum mgcp_trunk_type ttype, unsigned int nr);
 struct mgcp_trunk *mgcp_trunk_by_name(const struct mgcp_config *cfg, const char *epname);
-int e1_trunk_nr_from_epname(const char *epname);
+int e1_trunk_nr_from_epname(unsigned int *trunk_nr, const char *epname);
 struct mgcp_trunk *mgcp_trunk_by_line_num(const struct mgcp_config *cfg, unsigned int num);
 
 /* The virtual trunk is always created on trunk id 0 for historical reasons,
diff --git a/src/libosmo-mgcp/mgcp_endp.c b/src/libosmo-mgcp/mgcp_endp.c
index ddfd6cf..ae376b0 100644
--- a/src/libosmo-mgcp/mgcp_endp.c
+++ b/src/libosmo-mgcp/mgcp_endp.c
@@ -58,7 +58,7 @@
 }
 
 /* Generate E1 endpoint name from given numeric parameters */
-static char *gen_e1_epname(void *ctx, const char *domain, uint8_t trunk_nr,
+static char *gen_e1_epname(void *ctx, const char *domain, unsigned int trunk_nr,
 			   uint8_t ts_nr, uint8_t ss_nr)
 {
 	unsigned int rate;
diff --git a/src/libosmo-mgcp/mgcp_ratectr.c b/src/libosmo-mgcp/mgcp_ratectr.c
index af1526a..1f8b233 100644
--- a/src/libosmo-mgcp/mgcp_ratectr.c
+++ b/src/libosmo-mgcp/mgcp_ratectr.c
@@ -195,7 +195,7 @@
 		    rate_ctr_group_alloc(trunk, &mgcp_crcx_ctr_group_desc, crcx_rate_ctr_index);
 		if (!ratectr->mgcp_crcx_ctr_group)
 			return -EINVAL;
-		snprintf(ctr_name, sizeof(ctr_name), "%s-%d:crcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%u:crcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
 			 trunk->trunk_nr);
 		rate_ctr_group_set_name(ratectr->mgcp_crcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_crcx_ctr_group, free_rate_counter_group);
@@ -206,7 +206,7 @@
 		    rate_ctr_group_alloc(trunk, &mgcp_mdcx_ctr_group_desc, mdcx_rate_ctr_index);
 		if (!ratectr->mgcp_mdcx_ctr_group)
 			return -EINVAL;
-		snprintf(ctr_name, sizeof(ctr_name), "%s-%d:mdcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%u:mdcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
 			 trunk->trunk_nr);
 		rate_ctr_group_set_name(ratectr->mgcp_mdcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_mdcx_ctr_group, free_rate_counter_group);
@@ -217,7 +217,7 @@
 		    rate_ctr_group_alloc(trunk, &mgcp_dlcx_ctr_group_desc, dlcx_rate_ctr_index);
 		if (!ratectr->mgcp_dlcx_ctr_group)
 			return -EINVAL;
-		snprintf(ctr_name, sizeof(ctr_name), "%s-%d:dlcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%u:dlcx", mgcp_trunk_type_strs_str(trunk->trunk_type),
 			 trunk->trunk_nr);
 		rate_ctr_group_set_name(ratectr->mgcp_dlcx_ctr_group, ctr_name);
 		talloc_set_destructor(ratectr->mgcp_dlcx_ctr_group, free_rate_counter_group);
@@ -228,7 +228,7 @@
 								   all_rtp_conn_rate_ctr_index);
 		if (!ratectr->all_rtp_conn_stats)
 			return -EINVAL;
-		snprintf(ctr_name, sizeof(ctr_name), "%s-%d:rtp_conn", mgcp_trunk_type_strs_str(trunk->trunk_type),
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%u:rtp_conn", mgcp_trunk_type_strs_str(trunk->trunk_type),
 			 trunk->trunk_nr);
 		rate_ctr_group_set_name(ratectr->all_rtp_conn_stats, ctr_name);
 		talloc_set_destructor(ratectr->all_rtp_conn_stats, free_rate_counter_group);
@@ -240,7 +240,7 @@
 		ratectr->e1_stats = rate_ctr_group_alloc(trunk, &e1_rate_ctr_group_desc, mdcx_rate_ctr_index);
 		if (!ratectr->e1_stats)
 			return -EINVAL;
-		snprintf(ctr_name, sizeof(ctr_name), "%s-%d:e1", mgcp_trunk_type_strs_str(trunk->trunk_type),
+		snprintf(ctr_name, sizeof(ctr_name), "%s-%u:e1", mgcp_trunk_type_strs_str(trunk->trunk_type),
 			 trunk->trunk_nr);
 		rate_ctr_group_set_name(ratectr->e1_stats, ctr_name);
 		talloc_set_destructor(ratectr->e1_stats, free_rate_counter_group);
diff --git a/src/libosmo-mgcp/mgcp_trunk.c b/src/libosmo-mgcp/mgcp_trunk.c
index 08f99b3..27663b4 100644
--- a/src/libosmo-mgcp/mgcp_trunk.c
+++ b/src/libosmo-mgcp/mgcp_trunk.c
@@ -40,7 +40,7 @@
  *  \param[in] ttype trunk type.
  *  \param[in] nr trunk number.
  *  \returns pointer to allocated trunk, NULL on failure. */
-struct mgcp_trunk *mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr)
+struct mgcp_trunk *mgcp_trunk_alloc(struct mgcp_config *cfg, enum mgcp_trunk_type ttype, unsigned int nr)
 {
 	struct mgcp_trunk *trunk;
 
@@ -171,7 +171,7 @@
  *  \param[in] ttype trunk type.
  *  \param[in] nr trunk number.
  *  \returns pointer to trunk configuration, NULL on error. */
-struct mgcp_trunk *mgcp_trunk_by_num(const struct mgcp_config *cfg, enum mgcp_trunk_type ttype, int nr)
+struct mgcp_trunk *mgcp_trunk_by_num(const struct mgcp_config *cfg, enum mgcp_trunk_type ttype, unsigned int nr)
 {
 	struct mgcp_trunk *trunk;
 
@@ -184,9 +184,9 @@
 }
 
 /* Made public for unit-testing, do not use from outside this file */
-int e1_trunk_nr_from_epname(const char *epname)
+int e1_trunk_nr_from_epname(unsigned int *trunk_nr, const char *epname)
 {
-	unsigned long int trunk_nr;
+	unsigned long trunk_nr_temp;
 	size_t prefix_len;
 	char *str_trunk_nr_end;
 
@@ -195,13 +195,15 @@
 		return -EINVAL;
 
 	errno = 0;
-	trunk_nr = strtoul(epname + prefix_len, &str_trunk_nr_end, 10);
-	if (errno == ERANGE || trunk_nr > 64
+	trunk_nr_temp = strtoul(epname + prefix_len, &str_trunk_nr_end, 10);
+	if (errno == ERANGE || trunk_nr_temp > 64
 	    || epname + prefix_len == str_trunk_nr_end
 	    || str_trunk_nr_end[0] != '/')
 		return -EINVAL;
-	else
-		return trunk_nr;
+	else {
+		*trunk_nr = (unsigned int)trunk_nr_temp;
+		return 0;
+	}
 }
 
 /*! Find a trunk by the trunk prefix in the endpoint name.
@@ -212,7 +214,8 @@
 {
 	size_t prefix_len;
 	char epname_lc[MGCP_ENDPOINT_MAXLEN];
-	int trunk_nr;
+	unsigned int trunk_nr;
+	int rc;
 
 	osmo_str_tolower_buf(epname_lc, sizeof(epname_lc), epname);
 	epname = epname_lc;
@@ -222,8 +225,8 @@
 		return mgcp_trunk_by_num(cfg, MGCP_TRUNK_VIRTUAL, MGCP_VIRT_TRUNK_ID);
 	}
 
-	trunk_nr = e1_trunk_nr_from_epname(epname);
-	if (trunk_nr >= 0)
+	rc = e1_trunk_nr_from_epname(&trunk_nr, epname);
+	if (rc == 0)
 		return mgcp_trunk_by_num(cfg, MGCP_TRUNK_E1, trunk_nr);
 
 	/* Earlier versions of osmo-mgw were accepting endpoint names
diff --git a/src/libosmo-mgcp/mgcp_vty.c b/src/libosmo-mgcp/mgcp_vty.c
index 369c1c1..6bc09d0 100644
--- a/src/libosmo-mgcp/mgcp_vty.c
+++ b/src/libosmo-mgcp/mgcp_vty.c
@@ -200,11 +200,11 @@
 }
 
 static void dump_endpoint(struct vty *vty, struct mgcp_endpoint *endp,
-			  int trunk_nr, enum mgcp_trunk_type trunk_type, int show_stats)
+			  unsigned int trunk_nr, enum mgcp_trunk_type trunk_type, int show_stats)
 {
 	struct mgcp_conn *conn;
 
-	vty_out(vty, "%s trunk %d endpoint %s:%s",
+	vty_out(vty, "%s trunk %u endpoint %s:%s",
 		trunk_type == MGCP_TRUNK_VIRTUAL ? "Virtual" : "E1", trunk_nr, endp->name, VTY_NEWLINE);
 	vty_out(vty, "   Availability: %s%s",
 		mgcp_endp_avail(endp) ? "available" : "not in service", VTY_NEWLINE);
@@ -382,7 +382,7 @@
 		/* If a trunk is given, search on that specific trunk only */
 		endp = mgcp_endp_by_name_trunk(NULL, epname, trunk);
 		if (!endp) {
-			vty_out(vty, "endpoint %s not configured on trunk %d%s", epname, trunk->trunk_nr, VTY_NEWLINE);
+			vty_out(vty, "endpoint %s not configured on trunk %u%s", epname, trunk->trunk_nr, VTY_NEWLINE);
 			return;
 		}
 	} else {
@@ -963,7 +963,7 @@
       "trunk <0-64>", "Configure a SS7 trunk\n" "Trunk Nr\n")
 {
 	struct mgcp_trunk *trunk;
-	int index = atoi(argv[0]);
+	unsigned int index = atoi(argv[0]);
 
 	trunk = mgcp_trunk_by_num(g_cfg, MGCP_TRUNK_E1, index);
 	if (!trunk) {
@@ -995,7 +995,7 @@
 		    && trunk->trunk_nr == MGCP_VIRT_TRUNK_ID)
 			continue;
 
-		vty_out(vty, " trunk %d%s", trunk->trunk_nr, VTY_NEWLINE);
+		vty_out(vty, " trunk %u%s", trunk->trunk_nr, VTY_NEWLINE);
 		vty_out(vty, "  line %u%s", trunk->e1.vty_line_nr, VTY_NEWLINE);
 		vty_out(vty, "  %ssdp audio-payload send-ptime%s",
 			trunk->audio_send_ptime ? "" : "no ", VTY_NEWLINE);
@@ -1335,7 +1335,7 @@
 	}
 
 	if (!trunk->endpoints) {
-		vty_out(vty, "%%Trunk %d has no endpoints allocated.%s",
+		vty_out(vty, "%%Trunk %u has no endpoints allocated.%s",
 			trunk->trunk_nr, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
@@ -1396,7 +1396,7 @@
 	}
 
 	if (!trunk->endpoints) {
-		vty_out(vty, "%%Trunk %d has no endpoints allocated.%s",
+		vty_out(vty, "%%Trunk %u has no endpoints allocated.%s",
 			trunk->trunk_nr, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
@@ -1463,7 +1463,7 @@
 	}
 
 	if (!trunk->endpoints) {
-		vty_out(vty, "%%Trunk %d has no endpoints allocated.%s",
+		vty_out(vty, "%%Trunk %u has no endpoints allocated.%s",
 			trunk->trunk_nr, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
@@ -1496,7 +1496,7 @@
 	}
 
 	if (!trunk->endpoints) {
-		vty_out(vty, "%%Trunk %d has no endpoints allocated.%s",
+		vty_out(vty, "%%Trunk %u has no endpoints allocated.%s",
 			trunk->trunk_nr, VTY_NEWLINE);
 		return CMD_WARNING;
 	}
@@ -1756,7 +1756,7 @@
 	llist_for_each_entry(trunk, &g_cfg->trunks, entry) {
 		if (mgcp_trunk_equip(trunk) != 0) {
 			LOGP(DLMGCP, LOGL_ERROR,
-			     "Failed to initialize trunk %d (%d endpoints)\n",
+			     "Failed to initialize trunk %u (%d endpoints)\n",
 			     trunk->trunk_nr, trunk->number_endpoints);
 			return -1;
 		}
diff --git a/src/osmo-mgw/mgw_main.c b/src/osmo-mgw/mgw_main.c
index 113f691..52a1622 100644
--- a/src/osmo-mgw/mgw_main.c
+++ b/src/osmo-mgw/mgw_main.c
@@ -235,7 +235,7 @@
 	/* reset endpoints */
 	if (reset_endpoints) {
 		LOGP(DLMGCP, LOGL_NOTICE,
-		     "Asked to reset endpoints: %d/%d\n",
+		     "Asked to reset endpoints: %u/%d\n",
 		     reset_trunk->trunk_nr, reset_trunk->trunk_type);
 
 		/* reset flag */
diff --git a/tests/mgcp/mgcp_test.c b/tests/mgcp/mgcp_test.c
index 7397f5c..bcbcc02 100644
--- a/tests/mgcp/mgcp_test.c
+++ b/tests/mgcp/mgcp_test.c
@@ -2149,42 +2149,50 @@
 
 void test_e1_trunk_nr_from_epname()
 {
-	int trunk_nr;
+	unsigned int trunk_nr;
+	int rc;
 
 	/* Note: e1_trunk_nr_from_epname does not check the text
 	 * after the E1 trunk number, after the delimiter
 	 * character "/" arbitrary text may follow. */
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-0/s-1/su16-0");
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-0/s-1/su16-0");
 	OSMO_ASSERT(trunk_nr == 0);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-1/s-1/su16-0");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-1/s-1/su16-0");
 	OSMO_ASSERT(trunk_nr == 1);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-2/s-2/su16-0");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-2/s-2/su16-0");
 	OSMO_ASSERT(trunk_nr == 2);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-3/s-23/su32-0");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-3/s-23/su32-0");
 	OSMO_ASSERT(trunk_nr == 3);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-3/xxxxxxx");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-3/xxxxxxx");
 	OSMO_ASSERT(trunk_nr == 3);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-24/s-1/su16-0");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-24/s-1/su16-0");
 	OSMO_ASSERT(trunk_nr == 24);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-64/s-1/su16-0");
+	OSMO_ASSERT(rc == 0);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-64/s-1/su16-0");
 	OSMO_ASSERT(trunk_nr == 64);
+	OSMO_ASSERT(rc == 0);
 
 	/* The following endpoint strings should fail, either the
 	 * trunk number exceeds the valid range or the trunk prefix
 	 * is wrong. Also when the delimiter character "/" at the
 	 * end of the trunk is wrong the parsing should fail. */
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-65/s-1/su16-0");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1--1/s-1/su16-0");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
-	trunk_nr = e1_trunk_nr_from_epname("xxxxxx4zyz");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1+2/s-1/su16-0");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e2-24/s-1/su16-0");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
-	trunk_nr = e1_trunk_nr_from_epname("ds/e1-24s-1/su16-0");
-	OSMO_ASSERT(trunk_nr == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-65/s-1/su16-0");
+	OSMO_ASSERT(rc == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1--1/s-1/su16-0");
+	OSMO_ASSERT(rc == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "xxxxxx4zyz");
+	OSMO_ASSERT(rc == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1+2/s-1/su16-0");
+	OSMO_ASSERT(rc == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e2-24/s-1/su16-0");
+	OSMO_ASSERT(rc == -EINVAL);
+	rc = e1_trunk_nr_from_epname(&trunk_nr, "ds/e1-24s-1/su16-0");
+	OSMO_ASSERT(rc == -EINVAL);
 
 	return;
 }

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

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I5d0e1d76adb8c92d84331a0aca2496908e41d621
Gerrit-Change-Number: 24963
Gerrit-PatchSet: 2
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210721/a60af9cc/attachment.htm>


More information about the gerrit-log mailing list