Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/4110

to look at the new patch set (#3).

SI2q: cleanup UARFCN addition

* expand comments, fix typos
* constify parameter
* move try-add-adjust routine into separate function to facilitate
  further modifications
* remove excessive checks and unnecessary return values
* move (UARFCN, Scrambling Code) tuple uniqueness check into separate
  function and use it early

Change-Id: Ia72f848dec40723510ca56868e08081804227d47
Related: OS#2357
---
M src/libbsc/rest_octets.c
M src/libbsc/system_information.c
2 files changed, 90 insertions(+), 88 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/10/4110/3

diff --git a/src/libbsc/rest_octets.c b/src/libbsc/rest_octets.c
index 09c4a90..b1516ec 100644
--- a/src/libbsc/rest_octets.c
+++ b/src/libbsc/rest_octets.c
@@ -232,7 +232,7 @@
 }
 
 /* Estimate how many bits it'll take to append single FDD UARFCN */
-static inline int append_utran_fdd_length(uint16_t u, int *sc, size_t sc_len, 
size_t length)
+static inline int append_utran_fdd_length(uint16_t u, const int *sc, size_t 
sc_len, size_t length)
 {
        uint8_t chan_list[16] = { 0 };
        int tmp[sc_len], f0;
@@ -274,18 +274,42 @@
        return 21 + range1024_p(length);
 }
 
-/* Append multiple FDD UARFCNs */
-static inline int append_uarfcns(struct bitvec *bv, struct gsm_bts *bts, 
uint8_t budget)
+static inline int try_adding_uarfcn(struct bitvec *bv, struct gsm_bts *bts, 
uint16_t uarfcn,
+                                   uint8_t num_sc, uint8_t start_pos, uint8_t 
budget)
 {
-       const uint16_t *u = bts->si_common.data.uarfcn_list, *sc = 
bts->si_common.data.scramble_list;
-       int i, j, k, rc, st = 0, a[bts->si_common.uarfcn_length];
+       int i, k, rc, a[bts->si_common.uarfcn_length];
+
+       if (budget < 23)
+               return -ENOMEM;
+
+       /* copy corresponding Scrambling Codes: range encoder make in-place 
modifications */
+       for (i = start_pos, k = 0; i < num_sc; a[k++] = 
bts->si_common.data.scramble_list[i++]);
+
+       /* estimate bit length requirements */
+       rc = append_utran_fdd_length(uarfcn, a, bts->si_common.uarfcn_length, 
k);
+       if (rc < 0)
+               return rc; /* range encoder failure */
+
+       if (budget - rc <= 0)
+               return -ENOMEM; /* we have ran out of budget in current SI2q */
+
+       /* compute next offset */
+       bts->u_offset += k;
+
+       return budget - append_utran_fdd(bv, uarfcn, a, k);
+}
+
+/* Append multiple FDD UARFCNs */
+static inline void append_uarfcns(struct bitvec *bv, struct gsm_bts *bts, 
uint8_t budget)
+{
+       const uint16_t *u = bts->si_common.data.uarfcn_list;
+       int i, rem = budget - 7, st = 0; /* account for constant bits right 
away */
        uint16_t cu = u[bts->u_offset]; /* caller ensures that length is 
positive */
-       uint8_t rem = budget - 7, offset_diff; /* account for constant bits 
right away */
 
        OSMO_ASSERT(budget <= SI2Q_MAX_LEN);
 
        if (budget <= 7)
-               return -ENOMEM;
+               return;
 
        /* 3G Neighbour Cell Description */
        bitvec_set_bit(bv, 1);
@@ -299,53 +323,24 @@
        /* No Bandwidth_FDD */
        bitvec_set_bit(bv, 0);
 
-       for (i = bts->u_offset; i < bts->si_common.uarfcn_length; i++) {
-               offset_diff = 0;
-               for (j = st, k = 0; j < i; j++) {
-                       a[k++] = sc[j]; /* copy corresponding SCs */
-                       offset_diff++; /* compute proper offset step */
-               }
+       for (i = bts->u_offset; i <= bts->si_common.uarfcn_length; i++)
                if (u[i] != cu) { /* we've reached new UARFCN */
-                       rc = append_utran_fdd_length(cu, a, 
bts->si_common.uarfcn_length, k);
-                       if (rc < 0) { /* estimate bit length requirements */
-                               return rc;
-                       }
+                       rem = try_adding_uarfcn(bv, bts, cu, i, st, rem);
+                       if (rem < 0)
+                               break;
 
-                       if (rem - rc <= 0)
-                               break; /* we have ran out of budget in current 
SI2q */
-                       else {
-                               rem -= append_utran_fdd(bv, cu, a, k);
-                               bts->u_offset += offset_diff;
-                       }
-                       cu = u[i];
-                       st = i; /* update start position */
+                       if (i < bts->si_common.uarfcn_length) {
+                               cu = u[i];
+                               st = i;
+                       } else
+                               break;
                }
-       }
-
-       if (rem > 22) { /* add last UARFCN not covered by previous cycle if it 
could possibly fit into budget */
-               offset_diff = 0;
-               for (i = st, k = 0; i < bts->si_common.uarfcn_length; i++) {
-                       a[k++] = sc[i];
-                       offset_diff++;
-               }
-               rc = append_utran_fdd_length(cu, a, 
bts->si_common.uarfcn_length, k);
-               if (rc < 0) {
-                       return rc;
-               }
-
-               if (rem - rc >= 0) {
-                       rem -= append_utran_fdd(bv, cu, a, k);
-                       bts->u_offset += offset_diff;
-               }
-       }
 
        /* stop bit - end of Repeated UTRAN FDD Neighbour Cells */
        bitvec_set_bit(bv, 0);
 
        /* UTRAN TDD Description */
        bitvec_set_bit(bv, 0);
-
-       return 0;
 }
 
 /* generate SI2quater rest octets: 3GPP TS 44.018 ยง 10.5.2.33b */
@@ -393,15 +388,9 @@
        bitvec_set_bit(&bv, 0);
 
        rc = SI2Q_MAX_LEN - (bv.cur_bit + 3);
-       if (rc > 0 && bts->si_common.uarfcn_length - bts->u_offset > 0) {
-               rc = append_uarfcns(&bv, bts, rc);
-               if (rc < 0) {
-                       LOGP(DRR, LOGL_ERROR, "SI2quater [%u/%u]: failed to 
append %zu UARFCNs due to range encoding "
-                            "failure: %s\n",
-                            bts->si2q_index, bts->si2q_count, 
bts->si_common.uarfcn_length, strerror(-rc));
-                       return rc;
-               }
-       } else /* No 3G Neighbour Cell Description */
+       if (rc > 0 && bts->si_common.uarfcn_length - bts->u_offset > 0)
+               append_uarfcns(&bv, bts, rc);
+       else /* No 3G Neighbour Cell Description */
                bitvec_set_bit(&bv, 0);
 
        /* No 3G Measurement Parameters Description */
diff --git a/src/libbsc/system_information.c b/src/libbsc/system_information.c
index 7bdb80a..761e848 100644
--- a/src/libbsc/system_information.c
+++ b/src/libbsc/system_information.c
@@ -182,7 +182,7 @@
        int rc = make_si2quaters(bts, true);
        uint8_t num = bts->si2q_index + 1; /* number of SI2quater messages */
 
-       /* N. B: si2q_num() should NEVER be called during actualSI2q rest 
octets generation
+       /* N. B: si2q_num() should NEVER be called during actual SI2q rest 
octets generation
           we're not re-entrant because of the following code: */
        bts->u_offset = 0;
        bts->e_offset = 0;
@@ -239,26 +239,40 @@
        return r;
 }
 
+/* Scrambling Code as defined in 3GPP TS 25.213 is 9 bit long so number below 
is unreacheable upper bound */
+#define SC_BOUND 600
+
+/* Find position for a given UARFCN (take SC into consideration if it's 
available) in a sorted list
+   N. B: we rely on the assumption that (uarfcn, scramble) tuple is unique in 
the lists */
+static int uarfcn_sc_pos(const struct gsm_bts *bts, uint16_t uarfcn, uint16_t 
scramble)
+{
+       const uint16_t *sc = bts->si_common.data.scramble_list;
+       uint16_t i, scramble0 = encode_fdd(scramble, false), scramble1 = 
encode_fdd(scramble, true);
+       for (i = 0; i < bts->si_common.uarfcn_length; i++)
+               if (uarfcn == bts->si_common.data.uarfcn_list[i]) {
+                       if (scramble < SC_BOUND) {
+                               if (scramble0 == sc[i] || scramble1 == sc[i])
+                                       return i;
+                       } else
+                               return i;
+               }
+
+       return -1;
+}
+
 int bts_uarfcn_del(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble)
 {
-       uint16_t sc0 = encode_fdd(scramble, false), sc1 = encode_fdd(scramble, 
true),
-               *ual = bts->si_common.data.uarfcn_list,
-               *scl = bts->si_common.data.scramble_list;
-       size_t len = bts->si_common.uarfcn_length, i;
-       for (i = 0; i < len; i++) {
-               if (arfcn == ual[i] && (sc0 == scl[i] || sc1 == scl[i])) {
-                       /* we rely on the assumption that (uarfcn, scramble)
-                          tuple is unique in the lists */
-                       if (i != len - 1) { /* move the tail if necessary */
-                               memmove(ual + i, ual + i + 1, 2 * (len - i + 
1));
-                               memmove(scl + i, scl + i + 1, 2 * (len - i + 
1));
-                       }
-                       break;
-               }
-       }
+       uint16_t *ual = bts->si_common.data.uarfcn_list, *scl = 
bts->si_common.data.scramble_list;
+       size_t len = bts->si_common.uarfcn_length;
+       int pos = uarfcn_sc_pos(bts, arfcn, scramble);
 
-       if (i == len)
+       if (pos < 0)
                return -EINVAL;
+
+       if (pos != len - 1) { /* move the tail if necessary */
+               memmove(ual + pos, ual + pos + 1, 2 * (len - pos + 1));
+               memmove(scl + pos, scl + pos + 1, 2 * (len - pos + 1));
+       }
 
        bts->si_common.uarfcn_length--;
        return 0;
@@ -267,29 +281,27 @@
 int bts_uarfcn_add(struct gsm_bts *bts, uint16_t arfcn, uint16_t scramble, 
bool diversity)
 {
        size_t len = bts->si_common.uarfcn_length, i, k = 0;
-       uint16_t scr, chk,
+       uint8_t si2q;
+       int pos = uarfcn_sc_pos(bts, arfcn, scramble);
+       uint16_t scr = diversity ? encode_fdd(scramble, true) : 
encode_fdd(scramble, false),
                *ual = bts->si_common.data.uarfcn_list,
-               *scl = bts->si_common.data.scramble_list,
-               scramble1 = encode_fdd(scramble, true),
-               scramble0 = encode_fdd(scramble, false);
-
-       scr = diversity ? scramble1 : scramble0;
-       chk = diversity ? scramble0 : scramble1;
+               *scl = bts->si_common.data.scramble_list;
 
        if (len == MAX_EARFCN_LIST)
                return -ENOMEM;
 
-       for (i = 0; i < len; i++) /* find the position of arfcn if any */
-               if (arfcn == ual[i])
-                       break;
+       if (pos >= 0)
+               return -EADDRINUSE;
 
-       for (k = 0; i < len; i++) {
-               if (arfcn == ual[i] && (scr == scl[i] || chk == scl[i]))
-                       return -EADDRINUSE;
+       /* find the suitable position for arfcn if any */
+       pos = uarfcn_sc_pos(bts, arfcn, SC_BOUND);
+       i = (pos < 0) ? len : pos;
+
+       for (k = 0; i < len; i++)
                if (scr > scl[i])
                        k = i + 1;
-       }
-       /* we keep lists sorted by scramble code:
+
+       /* we keep lists sorted by scramble code of a given UARFCN:
           insert into appropriate position and move the tail */
        if (len - k) {
                memmove(ual + k + 1, ual + k, (len - k) * 2);
@@ -299,9 +311,10 @@
        ual[k] = arfcn;
        scl[k] = scr;
        bts->si_common.uarfcn_length++;
+       si2q = si2q_num(bts);
 
-       if (si2q_num(bts) <= SI2Q_MAX_NUM) {
-               bts->si2q_count = si2q_num(bts) - 1;
+       if (si2q <= SI2Q_MAX_NUM) {
+               bts->si2q_count = si2q - 1;
                return 0;
        }
 

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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia72f848dec40723510ca56868e08081804227d47
Gerrit-PatchSet: 3
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Owner: Max <[email protected]>
Gerrit-Reviewer: Jenkins Builder

Reply via email to