<p>Harald Welte <strong>merged</strong> this change.</p><p><a href="https://gerrit.osmocom.org/11510">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">abis_rsl.c: fix uninitialized RSL cause issues<br><br>Separate the cause value passed to further functions from the log string.<br><br>The code tried to be nice by composing the RSL cause string and returning the<br>RSL cause at the same time, which falls on its face when the string composition<br>happens only within conditional logging.<br><br>Change-Id: Ibadd06102f162bca9182c39b77b0651568d3e6f8<br>---<br>M src/osmo-bsc/abis_rsl.c<br>1 file changed, 18 insertions(+), 15 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c</span><br><span>index 589d673..b86780d 100644</span><br><span>--- a/src/osmo-bsc/abis_rsl.c</span><br><span>+++ b/src/osmo-bsc/abis_rsl.c</span><br><span>@@ -172,21 +172,22 @@</span><br><span> return lchan->encr.key_len + 1;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* If the TLV contain an RSL Cause IE, return the RSL cause name and point *rsl_cause_pp at the cause</span><br><span style="color: hsl(0, 100%, 40%);">- * value. If there is no Cause IE, return NULL and write NULL to *rsl_cause_pp. If NULL is passed as</span><br><span style="color: hsl(0, 100%, 40%);">- * rsl_cause_pp, ignore it. Implementation choice: presence of a Cause IE cannot be indicated by a zero</span><br><span style="color: hsl(0, 100%, 40%);">- * cause, because that would mean RSL_ERR_RADIO_IF_FAIL; a pointer-to-pointer can return NULL or point to</span><br><span style="color: hsl(0, 100%, 40%);">- * a cause value. */</span><br><span style="color: hsl(0, 100%, 40%);">-static const char *rsl_cause_name(struct tlv_parsed *tp, const uint8_t **rsl_cause_pp)</span><br><span style="color: hsl(120, 100%, 40%);">+/* If the TLV contain an RSL Cause IE, return pointer to the cause value. If there is no Cause IE, return</span><br><span style="color: hsl(120, 100%, 40%);">+ * NULL. Implementation choice: presence of a Cause IE cannot be indicated by a zero cause, because that</span><br><span style="color: hsl(120, 100%, 40%);">+ * would mean RSL_ERR_RADIO_IF_FAIL; a pointer can return NULL or point to a cause value. */</span><br><span style="color: hsl(120, 100%, 40%);">+static const uint8_t *rsl_cause(struct tlv_parsed *tp)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ if (TLVP_PRESENT(tp, RSL_IE_CAUSE))</span><br><span style="color: hsl(120, 100%, 40%);">+ return (const uint8_t *)TLVP_VAL(tp, RSL_IE_CAUSE);</span><br><span style="color: hsl(120, 100%, 40%);">+ return NULL;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/* If the TLV contain an RSL Cause IE, return the RSL cause name; otherwise return "". */</span><br><span style="color: hsl(120, 100%, 40%);">+static const char *rsl_cause_name(struct tlv_parsed *tp)</span><br><span> {</span><br><span> static char buf[128];</span><br><span style="color: hsl(0, 100%, 40%);">- if (rsl_cause_pp)</span><br><span style="color: hsl(0, 100%, 40%);">- *rsl_cause_pp = NULL;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (TLVP_PRESENT(tp, RSL_IE_CAUSE)) {</span><br><span> const uint8_t *cause = TLVP_VAL(tp, RSL_IE_CAUSE);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rsl_cause_pp)</span><br><span style="color: hsl(0, 100%, 40%);">- *rsl_cause_pp = cause;</span><br><span> snprintf(buf, sizeof(buf), " (cause=%s [ %s])",</span><br><span> rsl_err_name(*cause),</span><br><span> osmo_hexdump(cause, TLVP_LEN(tp, RSL_IE_CAUSE)));</span><br><span>@@ -871,7 +872,8 @@</span><br><span> }</span><br><span> </span><br><span> rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));</span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "CHANNEL ACTIVATE NACK%s\n", rsl_cause_name(&tp, &cause_p));</span><br><span style="color: hsl(120, 100%, 40%);">+ cause_p = rsl_cause(&tp);</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "CHANNEL ACTIVATE NACK%s\n", rsl_cause_name(&tp));</span><br><span> </span><br><span> if (msg_for_osmocom_dyn_ts(msg))</span><br><span> osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_PDCH_ACT_NACK, (void*)cause_p);</span><br><span>@@ -889,8 +891,9 @@</span><br><span> const uint8_t *cause_p;</span><br><span> </span><br><span> rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));</span><br><span style="color: hsl(120, 100%, 40%);">+ cause_p = rsl_cause(&tp);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", rsl_cause_name(&tp, &cause_p));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", rsl_cause_name(&tp));</span><br><span> </span><br><span> rate_ctr_inc(&lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_CHAN_RF_FAIL]);</span><br><span> </span><br><span>@@ -1208,7 +1211,7 @@</span><br><span> rsl_tlv_parse(&tp, rslh->data, msgb_l2len(msg)-sizeof(*rslh));</span><br><span> </span><br><span> LOGP(DRSL, LOGL_ERROR, "%s ERROR REPORT%s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- gsm_trx_name(sign_link->trx), rsl_cause_name(&tp, NULL));</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_trx_name(sign_link->trx), rsl_cause_name(&tp));</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>@@ -1978,7 +1981,7 @@</span><br><span> </span><br><span> rsl_tlv_parse(&tv, dh->data, msgb_l2len(msg)-sizeof(*dh));</span><br><span> LOG_LCHAN(msg->lchan, LOGL_NOTICE, "Rx IPACC DLCX IND%s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- rsl_cause_name(&tv, NULL));</span><br><span style="color: hsl(120, 100%, 40%);">+ rsl_cause_name(&tv));</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/11510">change 11510</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/11510"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ibadd06102f162bca9182c39b77b0651568d3e6f8 </div>
<div style="display:none"> Gerrit-Change-Number: 11510 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>