neels has uploaded this change for review.

View Change

coverity: vty for AMR start mode: clarify range checks

Coverity complains about a possible overflow to smod = -1. Clarify by
some type sanity that this is actually not possible.

Related: CID#465560
Change-Id: I34ac83e1441a34fcec6b10c34e10ab6dffa37607
---
M src/osmo-bsc/bts_vty.c
1 file changed, 17 insertions(+), 8 deletions(-)

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

diff --git a/src/osmo-bsc/bts_vty.c b/src/osmo-bsc/bts_vty.c
index 1babd4d..9c68b75 100644
--- a/src/osmo-bsc/bts_vty.c
+++ b/src/osmo-bsc/bts_vty.c
@@ -2781,23 +2781,32 @@
 	struct amr_multirate_conf *mr = (full) ? &bts->mr_full: &bts->mr_half;
 	struct gsm48_multi_rate_conf *mr_conf =
 				(struct gsm48_multi_rate_conf *) mr->gsm48_ie;
-	int num = 0, i;
-
-	for (i = 0; i < ((full) ? 8 : 6); i++) {
+	unsigned int num = 0;
+	for (int i = 0; i < ((full) ? 8 : 6); i++) {
 		if ((mr->gsm48_ie[1] & (1 << i))) {
 			num++;
 		}
 	}
 
-	if (argv[0][0] == 'a' || num == 0) {
+	/* When the user passed "auto", set ICMI=0.
+	 * Otherwise, pass ICMI=1 and the start mode in smod.
+	 * (smod starts with index 0, while arguments and 'num' start with index 1).
+	 */
+	if (strcmp(argv[0], "auto") == 0 || num == 0) {
 		mr_conf->icmi = 0;
 		mr_conf->smod = 0;
 	} else {
+		int arg = atoi(argv[0]);
+		/* clarify for coverity: the VTY argument spec is (auto|1|2|3|4). */
+		OSMO_ASSERT(arg >= 1 && arg <= 4);
+
+		/* If there are less modes than the start mode the user is asking for, fall back to the last available
+		 * mode. (FIXME: should this complain and fail instead?)
+		 * 'num' is guaranteed to be > 0 here. */
+		num = OSMO_MIN(num, arg);
+
 		mr_conf->icmi = 1;
-		if (num < atoi(argv[0]))
-			mr_conf->smod = num - 1;
-		else
-			mr_conf->smod = atoi(argv[0]) - 1;
+		mr_conf->smod = num - 1;
 	}
 }
 

To view, visit change 39364. To unsubscribe, or for help writing mail filters, visit settings.