Change in osmo-pcu[master]: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used...

[pespin]

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/18223 )

Change subject: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used to exist beforehand
......................................................................

pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used to exist beforehand

Variable found is used to always call Guard() on MS to avoid possible
unexpected freeing regressions.

Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
---
M src/pdch.cpp
1 file changed, 25 insertions(+), 23 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  neels: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/pdch.cpp b/src/pdch.cpp
index 149ca1f..048577d 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -548,38 +548,41 @@
 	struct gprs_rlcmac_sba *sba;
 
 	if (request->ID.UnionType) {
-		struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;
-		struct gprs_rlcmac_dl_tbf *dl_tbf = NULL;
+		struct gprs_rlcmac_ul_tbf *ul_tbf;
+		struct gprs_rlcmac_dl_tbf *dl_tbf;
 		uint32_t tlli = request->ID.u.TLLI;
 		uint8_t ta = GSM48_TA_INVALID;
+		bool found = true;
 
 		GprsMs *ms = bts()->ms_by_tlli(tlli);
-		if (!ms)
+		if (!ms) {
+			found = false;
 			ms = bts()->ms_alloc(0, 0); /* ms class updated later */
+		}
 
 		/* Keep the ms, even if it gets idle temporarily */
 		GprsMs::Guard guard(ms);
-		ul_tbf = ms->ul_tbf();
-		dl_tbf = ms->dl_tbf();
-		ta = ms->ta();
 
-		/* We got a RACH so the MS was in packet idle mode and thus
-		 * didn't have any active TBFs */
-		if (ul_tbf) {
-			LOGPTBFUL(ul_tbf, LOGL_NOTICE,
-				  "Got RACH from TLLI=0x%08x while TBF still exists. Killing pending UL TBF\n",
-				  tlli);
-			tbf_free(ul_tbf);
-			ul_tbf = NULL;
+		if (found) {
+			ul_tbf = ms->ul_tbf();
+			dl_tbf = ms->dl_tbf();
+			ta = ms->ta();
+			/* We got a RACH so the MS was in packet idle mode and thus
+			 * didn't have any active TBFs */
+			if (ul_tbf) {
+				LOGPTBFUL(ul_tbf, LOGL_NOTICE,
+					  "Got RACH from TLLI=0x%08x while TBF still exists. Killing pending UL TBF\n",
+					  tlli);
+				tbf_free(ul_tbf);
+			}
+			if (dl_tbf) {
+				LOGPTBFUL(dl_tbf, LOGL_NOTICE,
+					  "Got RACH from TLLI=0x%08x while TBF still exists. Release pending DL TBF\n",
+					  tlli);
+				tbf_free(dl_tbf);
+			}
 		}
 
-		if (dl_tbf) {
-			LOGPTBFUL(dl_tbf, LOGL_NOTICE,
-				  "Got RACH from TLLI=0x%08x while TBF still exists. Release pending DL TBF\n",
-				  tlli);
-			tbf_free(dl_tbf);
-			dl_tbf = NULL;
-		}
 		LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF "
 			"in packet resource request of single "
 			"block, so we provide one:\n");
@@ -606,12 +609,11 @@
 		if (!ms->ms_class())
 			LOGP(DRLCMAC, LOGL_NOTICE, "MS does not give us a class.\n");
 		if (ms->egprs_ms_class())
-			LOGP(DRLCMAC, LOGL_NOTICE,
+			LOGP(DRLCMAC, LOGL_INFO,
 				"MS supports EGPRS multislot class %d.\n",
 				ms->egprs_ms_class());
 
 		ul_tbf = tbf_alloc_ul(bts_data(), ms, trx_no(), tlli, ta);
-
 		if (!ul_tbf) {
 			handle_tbf_reject(bts_data(), ms, tlli,
 				trx_no(), ts_no);

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18223
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
Gerrit-Change-Number: 18223
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200514/7e964ae7/attachment.htm>