Review at  https://gerrit.osmocom.org/6630

HO: fix minor issues found by coverity

handover_decision_2:
- lchan->conn should never be NULL, but if it is, don't crash in LOGPHO*
  macros.
- make certain to avoid modulo-zero. It's cosmetic since that config item is
  not allowed to be zero.

handover_test:
- check return value of gsm_generate_si().
- safer evaluation of main()'s argv.

Fixes: CID#182929, CID#182928, CID#182927, CID#182926
Change-Id: I68e8ed3a1e8659edb47ac3e8a10508f17a3f5670
---
M src/libbsc/handover_decision_2.c
M tests/handover/handover_test.c
2 files changed, 21 insertions(+), 16 deletions(-)


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

diff --git a/src/libbsc/handover_decision_2.c b/src/libbsc/handover_decision_2.c
index 752b0d5..0135020 100644
--- a/src/libbsc/handover_decision_2.c
+++ b/src/libbsc/handover_decision_2.c
@@ -46,7 +46,7 @@
             lchan->ts->nr, \
             lchan->nr, \
             gsm_pchan_name(lchan->ts->pchan), \
-            bsc_subscr_name(lchan->conn->bsub), \
+            bsc_subscr_name(lchan->conn? lchan->conn->bsub : NULL), \
             ## args)
 
 #define LOGPHOLCHANTOBTS(lchan, new_bts, level, fmt, args...) \
@@ -57,7 +57,7 @@
             lchan->nr, \
             gsm_pchan_name(lchan->ts->pchan), \
             new_bts->nr, \
-            bsc_subscr_name(lchan->conn->bsub), \
+            bsc_subscr_name(lchan->conn? lchan->conn->bsub : NULL), \
             ## args)
 
 #define REQUIREMENT_A_TCHF     0x01
@@ -1114,6 +1114,7 @@
        struct gsm_lchan *lchan = mr->lchan;
        struct gsm_bts *bts = lchan->ts->trx->bts;
        int av_rxlev = -EINVAL, av_rxqual = -EINVAL;
+       unsigned int pwr_interval;
 
        /* we currently only do handover for TCH channels */
        switch (mr->lchan->type) {
@@ -1234,8 +1235,12 @@
                return;
        }
 
+       /* pwr_interval's range is 1-99, clarifying that no div-zero shall 
happen in modulo below: */
+       pwr_interval = ho_get_hodec2_pwr_interval(bts->ho);
+       OSMO_ASSERT(pwr_interval);
+
        /* try handover to a better cell */
-       if (av_rxlev >= 0 && (mr->nr % ho_get_hodec2_pwr_interval(bts->ho)) == 
0) {
+       if (av_rxlev >= 0 && (mr->nr % pwr_interval) == 0) {
                LOGPHOLCHAN(lchan, LOGL_INFO, "Looking whether a cell has 
better RXLEV\n");
                global_ho_reason = HO_REASON_BETTER_CELL;
                find_alternative_lchan(lchan, false);
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index e8716f2..d1f75ba 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -1223,7 +1223,6 @@
        test_case_26,
        test_case_27,
        test_case_28,
-       NULL
 };
 
 static const struct log_info_cat log_categories[] = {
@@ -1269,21 +1268,21 @@
        int bts_num = 0;
        struct gsm_lchan *lchan[256];
        int lchan_num = 0;
-       int test_count = 0;
        int i;
        int algorithm;
        struct bsc_api bsc_api = {};
+       int test_case_i;
+       int last_test_i;
 
-       for (i = 0; test_cases[i]; i++)
-               test_count++;
+       test_case_i = argc > 1? atoi(argv[1]) : -1;
+       last_test_i = ARRAY_SIZE(test_cases) - 1;
 
-       if (argc <= 1 || atoi(argv[1]) >= test_count) {
-               for (i = 0; test_cases[i]; i++) {
+       if (test_case_i < 0 || test_case_i > last_test_i) {
+               for (i = 0; i <= last_test_i; i++) {
                        printf("Test #%d (algorithm %s):\n%s\n", i,
                                test_cases[i][0], test_cases[i][1]);
                }
-               printf("\nPlease specify test case number 0..%d\n",
-                       test_count - 1);
+               printf("\nPlease specify test case number 0..%d\n", 
last_test_i);
                return EXIT_FAILURE;
        }
 
@@ -1320,11 +1319,11 @@
 
        bts_model_sysmobts_init();
 
-       test_case = test_cases[atoi(argv[1])];
+       test_case = test_cases[test_case_i];
 
        fprintf(stderr, "--------------------\n");
        fprintf(stderr, "Performing the following test %d (algorithm %s):\n%s",
-               atoi(argv[1]), test_case[0], test_case[1]);
+               test_case_i, test_case[0], test_case[1]);
        algorithm = atoi(test_case[0]);
        test_case += 2;
        fprintf(stderr, "--------------------\n");
@@ -1343,9 +1342,10 @@
                                "TS(1-4) are TCH/F, TS(5-6) are TCH/H)\n", n);
                        for (i = 0; i < n; i++)
                                bts[bts_num + i] = create_bts(arfcn++);
-                       for (i = 0; i < n; i++)
-                               gsm_generate_si(bts[bts_num + i],
-                                       SYSINFO_TYPE_2);
+                       for (i = 0; i < n; i++) {
+                               if (gsm_generate_si(bts[bts_num + i], 
SYSINFO_TYPE_2))
+                                       fprintf(stderr, "Error generating 
SI2\n");
+                       }
                        bts_num += n;
                        test_case += 2;
                } else

-- 
To view, visit https://gerrit.osmocom.org/6630
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I68e8ed3a1e8659edb47ac3e8a10508f17a3f5670
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>

Reply via email to