TBF: cleanup state flag handling

 * introduce generic function to check whether particular flag was set
   for'a TBF and clear it if necessary. Use this instead of
   clear_poll_timeout_flag()
 * add function to explicitly set assignment and appropriate state flags

Overall this makes the code easier to read and debug.

Related: OS#1759
Change-Id: Ic4560280c72f91700f2e19c6c7f6658dc29625c2

5 files changed, 36 insertions, 33 deletions

diff --git a/src/bts.cpp b/src/bts.cpp
index 4bc792a5..37a9c80c 100644
--- a/src/bts.cpp
+++ b/src/bts.cpp
@@ -1031,23 +1031,19 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
 				tbf->direction == new_tbf->direction)
 			tbf_free(tbf);
 
-		if ((new_tbf->state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH))) {
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_CCCH)) {
 			/* We now know that the PACCH really existed */
 			LOGPTBF(new_tbf, LOGL_INFO,
 				"The TBF has been confirmed on the PACCH, "
 				"changed type from CCCH to PACCH\n");
-			new_tbf->state_flags &= ~(1 << GPRS_RLCMAC_FLAG_CCCH);
 			new_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
 		}
 		new_tbf->set_state(GPRS_RLCMAC_FLOW);
 		/* stop pending assignment timer */
 		new_tbf->t_stop(T0, "control acked (DL-TBF)");
-		if ((new_tbf->state_flags &
-			(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS))) {
-			new_tbf->state_flags &=
-				~(1 << GPRS_RLCMAC_FLAG_TO_DL_ASS);
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ASS))
 			LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered downlink assignment\n");
-		}
+
 		tbf_assign_control_ts(new_tbf);
 		return;
 	}
@@ -1068,12 +1064,9 @@ void gprs_rlcmac_pdch::rcv_control_ack(Packet_Control_Acknowledgement_t *packet,
 			tbf_free(tbf);
 
 		new_tbf->set_state(GPRS_RLCMAC_FLOW);
-		if ((new_tbf->state_flags &
-			(1 << GPRS_RLCMAC_FLAG_TO_UL_ASS))) {
-			new_tbf->state_flags &=
-				~(1 << GPRS_RLCMAC_FLAG_TO_UL_ASS);
+		if (new_tbf->check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ASS))
 			LOGPTBF(new_tbf, LOGL_NOTICE, "Recovered uplink assignment for UL\n");
-		}
+
 		tbf_assign_control_ts(new_tbf);
 		/* there might be LLC packets waiting in the queue, but the DL
 		 * TBF might have been released while the UL TBF has been
diff --git a/src/tbf.cpp b/src/tbf.cpp
index 9e3a8efc..6847e181 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -402,8 +402,7 @@ gprs_rlcmac_ul_tbf *tbf_alloc_ul(struct gprs_rlcmac_bts *bts,
 		return NULL;
 	}
 	tbf->m_contention_resolution_done = 1;
-	tbf->set_state(GPRS_RLCMAC_ASSIGN);
-	tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+	tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false);
 	T_START(tbf, T3169, bts->t3169, 0, "allocation (UL-TBF)", true);
 	tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
 	OSMO_ASSERT(tbf->ms());
@@ -1511,8 +1510,7 @@ struct gprs_rlcmac_ul_tbf *handle_tbf_reject(struct gprs_rlcmac_bts *bts,
 
 	llist_add(&ul_tbf->list(), &bts->bts->ul_tbfs());
 	ul_tbf->bts->tbf_ul_created();
-	ul_tbf->set_state(GPRS_RLCMAC_ASSIGN);
-	ul_tbf->state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+	ul_tbf->set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, false);
 
 	ul_tbf->set_ms(ms);
 	ul_tbf->update_ms(tlli, GPRS_RLCMAC_UL_TBF);
diff --git a/src/tbf.h b/src/tbf.h
index 943ec928..6d7edbcf 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -180,6 +180,8 @@ struct gprs_rlcmac_tbf {
 	bool state_is(enum gprs_rlcmac_tbf_state rhs) const;
 	bool state_is_not(enum gprs_rlcmac_tbf_state rhs) const;
 	void set_state(enum gprs_rlcmac_tbf_state new_state);
+	bool check_n_clear(uint8_t state_flag);
+	void set_assigned_on(uint8_t state_flag, bool check_ccch);
 	const char *state_name() const;
 
 	const char *name() const;
@@ -368,6 +370,17 @@ inline const char *gprs_rlcmac_tbf::state_name() const
 	return tbf_state_name[state];
 }
 
+/* Set assignment state and corrsponding flags */
+inline void gprs_rlcmac_tbf::set_assigned_on(uint8_t state_flag, bool check_ccch)
+{
+	set_state(GPRS_RLCMAC_ASSIGN);
+	if (check_ccch) {
+		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)))
+			state_flags |= (1 << state_flag);
+	} else
+		state_flags |= (1 << state_flag);
+}
+
 inline void gprs_rlcmac_tbf::set_state(enum gprs_rlcmac_tbf_state new_state)
 {
 	LOGP(DRLCMAC, LOGL_DEBUG, "%s changes state from %s to %s\n",
@@ -376,6 +389,16 @@ inline void gprs_rlcmac_tbf::set_state(enum gprs_rlcmac_tbf_state new_state)
 	state = new_state;
 }
 
+inline bool gprs_rlcmac_tbf::check_n_clear(uint8_t state_flag)
+{
+	if ((state_flags & (1 << state_flag))) {
+		state_flags &= ~(1 << state_flag);
+		return true;
+	}
+
+	return false;
+}
+
 inline LListHead<gprs_rlcmac_tbf>& gprs_rlcmac_tbf::list()
 {
 	return this->m_list;
@@ -451,7 +474,7 @@ struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
 	int rcvd_dl_ack(bool final_ack, unsigned first_bsn, struct bitvec *rbb);
 	struct msgb *create_dl_acked_block(uint32_t fn, uint8_t ts);
 	void trigger_ass(struct gprs_rlcmac_tbf *old_tbf);
-	void clear_poll_timeout_flag();
+
 	bool handle_ack_nack();
 	void request_dl_ack();
 	bool need_control_ts() const;
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index b871bc32..e3b0a9de 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -493,9 +493,7 @@ void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
 		old_tbf->was_releasing = old_tbf->state_is(GPRS_RLCMAC_WAIT_RELEASE);
 
 		/* change state */
-		set_state(GPRS_RLCMAC_ASSIGN);
-		if (!(state_flags & (1 << GPRS_RLCMAC_FLAG_CCCH)))
-			state_flags |= (1 << GPRS_RLCMAC_FLAG_PACCH);
+		set_assigned_on(GPRS_RLCMAC_FLAG_PACCH, true);
 
 		/* start timer */
 		T_START(this, T0, T_ASS_PACCH_SEC, 0, "assignment (PACCH)", true);
@@ -505,8 +503,7 @@ void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
 		was_releasing = state_is(GPRS_RLCMAC_WAIT_RELEASE);
 
 		/* change state */
-		set_state(GPRS_RLCMAC_ASSIGN);
-		state_flags |= (1 << GPRS_RLCMAC_FLAG_CCCH);
+		set_assigned_on(GPRS_RLCMAC_FLAG_CCCH, false);
 
 		/* send immediate assignment */
 		bts->snd_dl_ass(this, 0, imsi());
@@ -634,18 +631,12 @@ int gprs_rlcmac_dl_tbf::create_new_bsn(const uint32_t fn, GprsCodingScheme cs)
 	return bsn;
 }
 
-void gprs_rlcmac_dl_tbf::clear_poll_timeout_flag()
-{
-	state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK);
-}
-
 bool gprs_rlcmac_dl_tbf::handle_ack_nack()
 {
 	bool ack_recovered = false;
 
 	state_flags |= (1 << GPRS_RLCMAC_FLAG_DL_ACK);
-	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_DL_ACK))) {
-		clear_poll_timeout_flag();
+	if (check_n_clear(GPRS_RLCMAC_FLAG_TO_DL_ACK)) {
 		ack_recovered = true;
 	}
 
@@ -856,7 +847,7 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(
 			if (is_final)
 				T_START(this, T3191, bts_data()->t3191, 0, "final block (DL-TBF)", true);
 
-			clear_poll_timeout_flag();
+			state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_DL_ACK); /* clear poll timeout flag */
 
 			/* Clear request flag */
 			m_dl_ack_requested = false;
diff --git a/src/tbf_ul.cpp b/src/tbf_ul.cpp
index 45de7cdf..6442b8f9 100644
--- a/src/tbf_ul.cpp
+++ b/src/tbf_ul.cpp
@@ -96,10 +96,8 @@ int gprs_rlcmac_ul_tbf::assemble_forward_llc(const gprs_rlc_data *_data)
 
 bool gprs_rlcmac_ul_tbf::ctrl_ack_to_toggle()
 {
-	if ((state_flags & (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK))) {
-		state_flags &= ~(1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
+	if (check_n_clear(GPRS_RLCMAC_FLAG_TO_UL_ACK))
 		return true; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was set, now cleared */
-	}
 
 	state_flags |= (1 << GPRS_RLCMAC_FLAG_TO_UL_ACK);
 	return false; /* GPRS_RLCMAC_FLAG_TO_UL_ACK was unset, now set */