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
This commit is contained in:
Philipp Maier 2021-07-19 13:53:28 +02:00
parent 33d97f721d
commit d70eef6421
7 changed files with 63 additions and 52 deletions

View File

@ -22,7 +22,7 @@ struct mgcp_trunk {
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 {
};
};
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,

View File

@ -58,7 +58,7 @@ static char *gen_virtual_epname(void *ctx, const char *domain,
}
/* 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;

View File

@ -195,7 +195,7 @@ int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
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 @@ int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
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 @@ int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
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 @@ int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
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 @@ int mgcp_ratectr_trunk_alloc(struct mgcp_trunk *trunk)
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);

View File

@ -40,7 +40,7 @@ const struct value_string mgcp_trunk_type_strs[] = {
* \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 @@ int mgcp_trunk_equip(struct mgcp_trunk *trunk)
* \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 @@ struct mgcp_trunk *mgcp_trunk_by_num(const struct mgcp_config *cfg, enum mgcp_tr
}
/* 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 @@ int e1_trunk_nr_from_epname(const char *epname)
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 @@ struct mgcp_trunk *mgcp_trunk_by_name(const struct mgcp_config *cfg, const char
{
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 @@ struct mgcp_trunk *mgcp_trunk_by_name(const struct mgcp_config *cfg, const char
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

View File

@ -200,11 +200,11 @@ static void dump_rtp_end(struct vty *vty, struct mgcp_conn_rtp *conn)
}
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 @@ dump_mgcp_endpoint(struct vty *vty, struct mgcp_trunk *trunk, const char *epname
/* 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 @@ DEFUN(cfg_mgcp_trunk, cfg_mgcp_trunk_cmd,
"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 @@ static int config_write_trunk(struct vty *vty)
&& 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 @@ DEFUN(loop_conn,
}
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 @@ DEFUN(tap_rtp,
}
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 @@ DEFUN(free_endp, free_endp_cmd,
}
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 @@ DEFUN(reset_endp, reset_endp_cmd,
}
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 @@ int mgcp_parse_config(const char *config_file, struct mgcp_config *cfg,
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;
}

View File

@ -235,7 +235,7 @@ static int read_call_agent(struct osmo_fd *fd, unsigned int what)
/* 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 */

View File

@ -2149,42 +2149,50 @@ void test_conn_id_matching()
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;
}