Neels Hofmeyr has uploaded this change for review. ( 
https://gerrit.osmocom.org/11510


Change subject: abis_rsl.c: fix uninitialized RSL cause issues
......................................................................

abis_rsl.c: fix uninitialized RSL cause issues

Separate the cause value passed to further functions from the log string.

The code tried to be nice by composing the RSL cause string and returning the
RSL cause at the same time, which falls on its face when the string composition
happens only within conditional logging.

Change-Id: Ibadd06102f162bca9182c39b77b0651568d3e6f8
---
M src/osmo-bsc/abis_rsl.c
1 file changed, 18 insertions(+), 15 deletions(-)



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

diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 589d673..b86780d 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -172,21 +172,22 @@
        return lchan->encr.key_len + 1;
 }

-/* If the TLV contain an RSL Cause IE, return the RSL cause name and point 
*rsl_cause_pp at the cause
- * value. If there is no Cause IE, return NULL and write NULL to 
*rsl_cause_pp. If NULL is passed as
- * rsl_cause_pp, ignore it. Implementation choice: presence of a Cause IE 
cannot be indicated by a zero
- * cause, because that would mean RSL_ERR_RADIO_IF_FAIL; a pointer-to-pointer 
can return NULL or point to
- * a cause value. */
-static const char *rsl_cause_name(struct tlv_parsed *tp, const uint8_t 
**rsl_cause_pp)
+/* If the TLV contain an RSL Cause IE, return pointer to the cause value. If 
there is no Cause IE, return
+ * NULL. Implementation choice: presence of a Cause IE cannot be indicated by 
a zero cause, because that
+ * would mean RSL_ERR_RADIO_IF_FAIL; a pointer can return NULL or point to a 
cause value. */
+static const uint8_t *rsl_cause(struct tlv_parsed *tp)
+{
+       if (TLVP_PRESENT(tp, RSL_IE_CAUSE))
+               return (const uint8_t *)TLVP_VAL(tp, RSL_IE_CAUSE);
+       return NULL;
+}
+
+/* If the TLV contain an RSL Cause IE, return the RSL cause name; otherwise 
return "". */
+static const char *rsl_cause_name(struct tlv_parsed *tp)
 {
        static char buf[128];
-       if (rsl_cause_pp)
-               *rsl_cause_pp = NULL;
-
        if (TLVP_PRESENT(tp, RSL_IE_CAUSE)) {
                const uint8_t *cause = TLVP_VAL(tp, RSL_IE_CAUSE);
-               if (rsl_cause_pp)
-                       *rsl_cause_pp = cause;
                snprintf(buf, sizeof(buf), " (cause=%s [ %s])",
                         rsl_err_name(*cause),
                         osmo_hexdump(cause, TLVP_LEN(tp, RSL_IE_CAUSE)));
@@ -871,7 +872,8 @@
        }

        rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));
-       LOG_LCHAN(lchan, LOGL_ERROR, "CHANNEL ACTIVATE NACK%s\n", 
rsl_cause_name(&tp, &cause_p));
+       cause_p = rsl_cause(&tp);
+       LOG_LCHAN(lchan, LOGL_ERROR, "CHANNEL ACTIVATE NACK%s\n", 
rsl_cause_name(&tp));

        if (msg_for_osmocom_dyn_ts(msg))
                osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_PDCH_ACT_NACK, 
(void*)cause_p);
@@ -889,8 +891,9 @@
        const uint8_t *cause_p;

        rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));
+       cause_p = rsl_cause(&tp);

-       LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", rsl_cause_name(&tp, 
&cause_p));
+       LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", 
rsl_cause_name(&tp));

        rate_ctr_inc(&lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_CHAN_RF_FAIL]);

@@ -1208,7 +1211,7 @@
        rsl_tlv_parse(&tp, rslh->data, msgb_l2len(msg)-sizeof(*rslh));

        LOGP(DRSL, LOGL_ERROR, "%s ERROR REPORT%s\n",
-            gsm_trx_name(sign_link->trx), rsl_cause_name(&tp, NULL));
+            gsm_trx_name(sign_link->trx), rsl_cause_name(&tp));

        return 0;
 }
@@ -1978,7 +1981,7 @@

        rsl_tlv_parse(&tv, dh->data, msgb_l2len(msg)-sizeof(*dh));
        LOG_LCHAN(msg->lchan, LOGL_NOTICE, "Rx IPACC DLCX IND%s\n",
-                 rsl_cause_name(&tv, NULL));
+                 rsl_cause_name(&tv));

        return 0;
 }

--
To view, visit https://gerrit.osmocom.org/11510
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

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

Reply via email to