neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/39364?usp=email )
Change subject: coverity: vty for AMR start mode: clarify range checks ......................................................................
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; } }