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

change return type of page_subscriber() to void

We deliberately ignore errors from page_subscriber() so there is
no point in having a non-void return value.
Make this function log an error if paging failed.

Adjust an implementation override of base_grace_paging_request()
in the test suite, which got return value semantics wrong.
(Without this change, the test would start to fail because we're
now logging paging errors which the test doesn't expect).

Change-Id: Ie18c2ba53d2055d3eaff8c9ed939eb844af6dd2e
Related: I48f5efbcddd98e15256edfca06ba0ae6acb5bab1
---
M src/osmo-bsc/osmo_bsc_bssap.c
M tests/bssap/bssap_test.c
2 files changed, 12 insertions(+), 19 deletions(-)


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

diff --git a/src/osmo-bsc/osmo_bsc_bssap.c b/src/osmo-bsc/osmo_bsc_bssap.c
index 3cd6051..3f7e4f8 100644
--- a/src/osmo-bsc/osmo_bsc_bssap.c
+++ b/src/osmo-bsc/osmo_bsc_bssap.c
@@ -237,44 +237,42 @@
 
 /* Page a subscriber based on TMSI and LAC via the specified BTS.
  * The msc parameter is the MSC which issued the corresponding paging request.
- * Returns 1 if the paging request could be issued, 0 if not.
- * A negative return value indicates an error. */
-static int
+ * Log an error if paging failed. */
+static void
 page_subscriber(struct bsc_msc_data *msc, struct gsm_bts *bts,
     uint32_t tmsi, uint32_t lac, const char *mi_string, uint8_t chan_needed)
 {
        struct bsc_subscr *subscr;
        int ret;
 
+       LOGP(DMSC, LOGL_INFO, "Paging request from MSC BTS: %d IMSI: '%s' TMSI: 
'0x%x/%u' LAC: 0x%x\n",
+           bts->nr, mi_string, tmsi, tmsi, lac);
+
        subscr = 
bsc_subscr_find_or_create_by_imsi(msc->network->bsc_subscribers,
                                                   mi_string);
        if (!subscr) {
-               LOGP(DMSC, LOGL_ERROR, "Failed to allocate a subscriber for 
%s\n", mi_string);
-               return -1;
+               LOGP(DMSC, LOGL_ERROR, "Paging request failed: Could not 
allocate subscriber for %s\n", mi_string);
+               return;
        }
 
        subscr->lac = lac;
        subscr->tmsi = tmsi;
 
-       LOGP(DMSC, LOGL_INFO, "Paging request from MSC BTS: %d IMSI: '%s' TMSI: 
'0x%x/%u' LAC: 0x%x\n",
-           bts->nr, mi_string, tmsi, tmsi, lac);
-
        ret = bsc_grace_paging_request(msc->network->bsc_data->rf_ctrl->policy, 
subscr, chan_needed, msc, bts);
+       if (ret == 0)
+               LOGP(DMSC, LOGL_ERROR, "Paging request failed: BTS: %d IMSI: 
'%s' TMSI: '0x%x/%u' LAC: 0x%x\n",
+                    bts->nr, mi_string, tmsi, tmsi, lac);
 
        /* the paging code has grabbed its own references */
        bsc_subscr_put(subscr);
-
-       return ret;
 }
 
 static void
 page_all_bts(struct bsc_msc_data *msc, uint32_t tmsi, const char *mi_string, 
uint8_t chan_needed)
 {
        struct gsm_bts *bts;
-       llist_for_each_entry(bts, &msc->network->bts_list, list) {
-               /* ignore errors from page_subscriber(); try all BTS */
+       llist_for_each_entry(bts, &msc->network->bts_list, list)
                page_subscriber(msc, bts, tmsi, GSM_LAC_RESERVED_ALL_BTS, 
mi_string, chan_needed);
-       }
 }
 
 static void
@@ -292,7 +290,6 @@
                                        continue;
                                if (bts->cell_identity != id->cell_identity)
                                        continue;
-                               /* ignore errors from page_subscriber(); keep 
trying other BTS */
                                page_subscriber(msc, bts, tmsi, id->lai.lac, 
mi_string, chan_needed);
                                paged = 1;
                        }
@@ -324,7 +321,6 @@
                                continue;
                        if (bts->cell_identity != id->ci)
                                continue;
-                       /* ignore errors from page_subscriber(); keep trying 
other BTS */
                        page_subscriber(msc, bts, tmsi, id->lac, mi_string, 
chan_needed);
                        paged = 1;
                }
@@ -348,7 +344,6 @@
                llist_for_each_entry(bts, &msc->network->bts_list, list) {
                        if (bts->cell_identity != ci)
                                continue;
-                       /* ignore errors from page_subscriber(); keep trying 
other BTS */
                        page_subscriber(msc, bts, tmsi, 
GSM_LAC_RESERVED_ALL_BTS, mi_string, chan_needed);
                        paged = 1;
                }
@@ -373,7 +368,6 @@
                        llist_for_each_entry(bts, &msc->network->bts_list, 
list) {
                                if (bts->location_area_code != id->lac)
                                        continue;
-                               /* ignore errors from page_subscriber(); keep 
trying other BTS */
                                page_subscriber(msc, bts, tmsi, id->lac, 
mi_string, chan_needed);
                                paged = 1;
                        }
@@ -403,7 +397,6 @@
                llist_for_each_entry(bts, &msc->network->bts_list, list) {
                        if (bts->location_area_code != lac)
                                continue;
-                       /* ignore errors from page_subscriber(); keep trying 
other BTS */
                        page_subscriber(msc, bts, tmsi, lac, mi_string, 
chan_needed);
                        paged = 1;
                }
diff --git a/tests/bssap/bssap_test.c b/tests/bssap/bssap_test.c
index 5601e29..00d925d 100644
--- a/tests/bssap/bssap_test.c
+++ b/tests/bssap/bssap_test.c
@@ -50,7 +50,7 @@
        else
                fprintf(stderr, "BSC paging started with LAC %u\n", 
subscr->lac);
        OSMO_ASSERT(gl_expect_lac == subscr->lac);
-       return 0;
+       return 1; /* pretend one BTS was paged */
 }
 
 struct {

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie18c2ba53d2055d3eaff8c9ed939eb844af6dd2e
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <ssperl...@sysmocom.de>

Reply via email to