Change in osmo-bsc[master]: don't use rsl_cause_name() in LOGP statements

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Harald Welte gerrit-no-reply at lists.osmocom.org
Sun Oct 21 09:31:38 UTC 2018


Harald Welte has uploaded this change for review. ( https://gerrit.osmocom.org/11399


Change subject: don't use rsl_cause_name() in LOGP statements
......................................................................

don't use rsl_cause_name() in LOGP statements

rsl_cause_name() doesn't only return the name of the cause,
but also returns a pointer to the binary cause IE value.  If we
want to use that cause IE value, then we must not call rsl_cause_name()
from within a LOGP/DEBUGP statement, as this will only be executed
if the log level is high enough.

I actually believe the rsl_cause_name() should be side-effect free,
i.e. it should simply return a name and not also return a pointer
to the cause in an output pointer-pointer.  This way it's less easy
to fall into this trap.

Change-Id: I0ecabcd6435f3f74448d5d3350fe659d7df91a6d
Fixes: Coverity CID#188832, CID#188847
---
M src/osmo-bsc/abis_rsl.c
1 file changed, 6 insertions(+), 2 deletions(-)



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

diff --git a/src/osmo-bsc/abis_rsl.c b/src/osmo-bsc/abis_rsl.c
index 589d673..2443bb5 100644
--- a/src/osmo-bsc/abis_rsl.c
+++ b/src/osmo-bsc/abis_rsl.c
@@ -861,6 +861,7 @@
 	struct tlv_parsed tp;
 	struct gsm_lchan *lchan = msg->lchan;
 	const uint8_t *cause_p;
+	const char *cause_name;
 
 	rate_ctr_inc(&msg->lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_CHAN_ACT_NACK]);
 
@@ -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_name = rsl_cause_name(&tp, &cause_p);
+	LOG_LCHAN(lchan, LOGL_ERROR, "CHANNEL ACTIVATE NACK%s\n", cause_name);
 
 	if (msg_for_osmocom_dyn_ts(msg))
 		osmo_fsm_inst_dispatch(lchan->ts->fi, TS_EV_PDCH_ACT_NACK, (void*)cause_p);
@@ -887,10 +889,12 @@
 	struct gsm_lchan *lchan = msg->lchan;
 	struct tlv_parsed tp;
 	const uint8_t *cause_p;
+	const char *cause_name;
 
 	rsl_tlv_parse(&tp, dh->data, msgb_l2len(msg)-sizeof(*dh));
+	cause_name = rsl_cause_name(&tp, &cause_p);
 
-	LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", rsl_cause_name(&tp, &cause_p));
+	LOG_LCHAN(lchan, LOGL_ERROR, "CONNECTION FAIL%s\n", cause_name);
 
 	rate_ctr_inc(&lchan->ts->trx->bts->bts_ctrs->ctr[BTS_CTR_CHAN_RF_FAIL]);
 

-- 
To view, visit https://gerrit.osmocom.org/11399
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: I0ecabcd6435f3f74448d5d3350fe659d7df91a6d
Gerrit-Change-Number: 11399
Gerrit-PatchSet: 1
Gerrit-Owner: Harald Welte <laforge at gnumonks.org>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181021/b3a227c6/attachment.htm>


More information about the gerrit-log mailing list