[PATCH] osmo-bts[master]: Remove duplicated code

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/.

Max gerrit-no-reply at lists.osmocom.org
Mon Oct 17 14:40:16 UTC 2016


Hello Harald Welte, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/1046

to look at the new patch set (#5).

Remove duplicated code

* Having duplicated code to fill in fn & tn values makes it harder to read
  and modify static gsmtap_p* functions. Fix this by removing the
  duplication and moving the common code one level up.

* Remove lchan activation/deactivation related code duplication to
  facilitate future use for dynamic CCCH re-activation.

Change-Id: Id0d3b19dbfaa16d1734321a07a6eb0355bfd77c9
---
M include/osmo-bts/oml.h
M src/common/l1sap.c
M src/common/oml.c
M src/osmo-bts-litecell15/lc15bts_vty.c
M src/osmo-bts-litecell15/oml.c
M src/osmo-bts-octphy/l1_oml.c
M src/osmo-bts-sysmo/oml.c
M src/osmo-bts-sysmo/sysmobts_vty.c
M tests/handover/handover_test.c
M tests/stubs.c
10 files changed, 51 insertions(+), 73 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/46/1046/5

diff --git a/include/osmo-bts/oml.h b/include/osmo-bts/oml.h
index 9f49444..1a47cde 100644
--- a/include/osmo-bts/oml.h
+++ b/include/osmo-bts/oml.h
@@ -18,6 +18,12 @@
 int oml_mo_statechg_ack(struct gsm_abis_mo *mo);
 int oml_mo_statechg_nack(struct gsm_abis_mo *mo, uint8_t nack_cause);
 
+void enqueue_rel_marker(struct gsm_lchan *lchan);
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan);
+int lchan_activate(struct gsm_lchan *lchan);
+int lchan_deactivate(struct gsm_lchan *lchan);
+int lchan_deactivate_sacch(struct gsm_lchan *lchan);
+
 /* Change the state and send STATE CHG REP */
 int oml_mo_state_chg(struct gsm_abis_mo *mo, int op_state, int avail_state);
 
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index f3e620e..0b9930e 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -205,15 +205,13 @@
 
 /* send primitive as gsmtap */
 static int gsmtap_ph_data(struct osmo_phsap_prim *l1sap, uint8_t *chan_type,
-	uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, int *len)
+			  uint8_t *ss, uint32_t fn, uint8_t **data, int *len)
 {
 	struct msgb *msg = l1sap->oph.msg;
 	uint8_t chan_nr, link_id;
 
 	*data = msg->data + sizeof(struct osmo_phsap_prim);
 	*len = msg->len - sizeof(struct osmo_phsap_prim);
-	*fn = l1sap->u.data.fn;
-	*tn = L1SAP_CHAN2TS(l1sap->u.data.chan_nr);
 	chan_nr = l1sap->u.data.chan_nr;
 	link_id = l1sap->u.data.link_id;
 
@@ -234,7 +232,7 @@
 #warning Set BS_AG_BLKS_RES
 		/* The sapi depends on DSP configuration, not
 		 * on the actual SYSTEM INFORMATION 3. */
-		if (L1SAP_FN2CCCHBLOCK(*fn) >= 1)
+		if (L1SAP_FN2CCCHBLOCK(fn) >= 1)
 			*chan_type = GSMTAP_CHANNEL_PCH;
 		else
 			*chan_type = GSMTAP_CHANNEL_AGCH;
@@ -246,18 +244,16 @@
 }
 
 static int gsmtap_pdch(struct osmo_phsap_prim *l1sap, uint8_t *chan_type,
-	uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, int *len)
+		       uint8_t *ss, uint32_t fn, uint8_t **data, int *len)
 {
 	struct msgb *msg = l1sap->oph.msg;
 
 	*data = msg->data + sizeof(struct osmo_phsap_prim);
 	*len = msg->len - sizeof(struct osmo_phsap_prim);
-	*fn = l1sap->u.data.fn;
-	*tn = L1SAP_CHAN2TS(l1sap->u.data.chan_nr);
 
-	if (L1SAP_IS_PTCCH(*fn)) {
+	if (L1SAP_IS_PTCCH(fn)) {
 		*chan_type = GSMTAP_CHANNEL_PTCCH;
-		*ss = L1SAP_FN2PTCCHBLOCK(*fn);
+		*ss = L1SAP_FN2PTCCHBLOCK(fn);
 		if (l1sap->oph.primitive
 				== PRIM_OP_INDICATION) {
 			if ((*data[0]) == 7)
@@ -309,12 +305,14 @@
 		uplink = 0;
 		/* fall through */
 	case OSMO_PRIM(PRIM_PH_DATA, PRIM_OP_INDICATION):
+		fn = l1sap->u.data.fn;
+		tn = L1SAP_CHAN2TS(l1sap->u.data.chan_nr);
 		if (ts_is_pdch(&trx->ts[tn]))
-			rc = gsmtap_pdch(l1sap, &chan_type, &tn, &ss, &fn, &data,
-				&len);
+			rc = gsmtap_pdch(l1sap, &chan_type, &ss, fn, &data,
+					 &len);
 		else
-			rc = gsmtap_ph_data(l1sap, &chan_type, &tn, &ss, &fn,
-				&data, &len);
+			rc = gsmtap_ph_data(l1sap, &chan_type, &ss, fn, &data,
+					    &len);
 		break;
 	case OSMO_PRIM(PRIM_PH_RACH, PRIM_OP_INDICATION):
 		rc = gsmtap_ph_rach(l1sap, &chan_type, &tn, &ss, &fn, &data,
diff --git a/src/common/oml.c b/src/common/oml.c
index 690a81d..6c42f10 100644
--- a/src/common/oml.c
+++ b/src/common/oml.c
@@ -453,6 +453,20 @@
 	return 0;
 }
 
+int lchan_deactivate(struct gsm_lchan *lchan)
+{
+	lchan_set_state(lchan, LCHAN_S_REL_REQ);
+	lchan->ciph_state = 0;
+	enqueue_rel_marker(lchan);
+	return 0;
+}
+
+int lchan_deactivate_sacch(struct gsm_lchan *lchan)
+{
+	enqueue_sacch_rel_marker(lchan);
+	return 0;
+}
+
 /* 8.6.1 Set BTS Attributes has been received */
 static int oml_rx_set_bts_attr(struct gsm_bts *bts, struct msgb *msg)
 {
diff --git a/src/osmo-bts-litecell15/lc15bts_vty.c b/src/osmo-bts-litecell15/lc15bts_vty.c
index c5d404c..e4773b4 100644
--- a/src/osmo-bts-litecell15/lc15bts_vty.c
+++ b/src/osmo-bts-litecell15/lc15bts_vty.c
@@ -46,15 +46,12 @@
 #include <osmo-bts/gsm_data.h>
 #include <osmo-bts/phy_link.h>
 #include <osmo-bts/logging.h>
+#include <osmo-bts/oml.h>
 #include <osmo-bts/vty.h>
 
 #include "lc15bts.h"
 #include "l1_if.h"
 #include "utils.h"
-
-
-extern int lchan_activate(struct gsm_lchan *lchan);
-extern int lchan_deactivate(struct gsm_lchan *lchan);
 
 #define TRX_STR "Transceiver related commands\n" "TRX number\n"
 
diff --git a/src/osmo-bts-litecell15/oml.c b/src/osmo-bts-litecell15/oml.c
index 634c236..5a5d452 100644
--- a/src/osmo-bts-litecell15/oml.c
+++ b/src/osmo-bts-litecell15/oml.c
@@ -262,8 +262,6 @@
 }
 #endif
 
-int lchan_activate(struct gsm_lchan *lchan);
-
 static int opstart_compl(struct gsm_abis_mo *mo, struct msgb *l1_msg)
 {
 	GsmL1_Prim_t *l1p = msgb_l1prim(l1_msg);
@@ -1645,7 +1643,7 @@
 	return res;
 }
 
-static void enqueue_rel_marker(struct gsm_lchan *lchan)
+void enqueue_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -1655,15 +1653,7 @@
 	queue_sapi_command(lchan, cmd);
 }
 
-int lchan_deactivate(struct gsm_lchan *lchan)
-{
-	lchan_set_state(lchan, LCHAN_S_REL_REQ);
-	lchan->ciph_state = 0; /* FIXME: do this in common/\*.c */
-	enqueue_rel_marker(lchan);
-	return 0;
-}
-
-static void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -1671,12 +1661,6 @@
 	cmd = talloc_zero(lchan->ts->trx, struct sapi_cmd);
 	cmd->type = SAPI_CMD_SACCH_REL_MARKER;
 	queue_sapi_command(lchan, cmd);
-}
-
-static int lchan_deactivate_sacch(struct gsm_lchan *lchan)
-{
-	enqueue_sacch_rel_marker(lchan);
-	return 0;
 }
 
 /* callback from OML */
diff --git a/src/osmo-bts-octphy/l1_oml.c b/src/osmo-bts-octphy/l1_oml.c
index 74853bf..e7626e8 100644
--- a/src/osmo-bts-octphy/l1_oml.c
+++ b/src/osmo-bts-octphy/l1_oml.c
@@ -873,7 +873,7 @@
  * RSL DEACTIVATE SACCH
  ***********************************************************************/
 
-static void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -881,12 +881,6 @@
 	cmd = talloc_zero(lchan->ts->trx, struct sapi_cmd);
 	cmd->type = SAPI_CMD_SACCH_REL_MARKER;
 	queue_sapi_command(lchan, cmd);
-}
-
-static int lchan_deactivate_sacch(struct gsm_lchan *lchan)
-{
-	enqueue_sacch_rel_marker(lchan);
-	return 0;
 }
 
 int l1if_rsl_deact_sacch(struct gsm_lchan *lchan)
@@ -902,7 +896,7 @@
  * RSL CHANNEL RELEASE
  ***********************************************************************/
 
-static void enqueue_rel_marker(struct gsm_lchan *lchan)
+void enqueue_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -910,14 +904,6 @@
 	cmd = talloc_zero(lchan->ts->trx, struct sapi_cmd);
 	cmd->type = SAPI_CMD_REL_MARKER;
 	queue_sapi_command(lchan, cmd);
-}
-
-static int lchan_deactivate(struct gsm_lchan *lchan)
-{
-	lchan_set_state(lchan, LCHAN_S_REL_REQ);
-	lchan->ciph_state = 0;	/* FIXME: do this in common *.c */
-	enqueue_rel_marker(lchan);
-	return 0;
 }
 
 int l1if_rsl_chan_rel(struct gsm_lchan *lchan)
diff --git a/src/osmo-bts-sysmo/oml.c b/src/osmo-bts-sysmo/oml.c
index c1f1e0b..7a07ad1 100644
--- a/src/osmo-bts-sysmo/oml.c
+++ b/src/osmo-bts-sysmo/oml.c
@@ -261,8 +261,6 @@
 }
 #endif
 
-int lchan_activate(struct gsm_lchan *lchan);
-
 static int opstart_compl(struct gsm_abis_mo *mo, struct msgb *l1_msg)
 {
 	GsmL1_Prim_t *l1p = msgb_l1prim(l1_msg);
@@ -1665,7 +1663,7 @@
 	return res;
 }
 
-static void enqueue_rel_marker(struct gsm_lchan *lchan)
+void enqueue_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -1675,15 +1673,7 @@
 	queue_sapi_command(lchan, cmd);
 }
 
-int lchan_deactivate(struct gsm_lchan *lchan)
-{
-	lchan_set_state(lchan, LCHAN_S_REL_REQ);
-	lchan->ciph_state = 0; /* FIXME: do this in common/\*.c */
-	enqueue_rel_marker(lchan);
-	return 0;
-}
-
-static void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
 {
 	struct sapi_cmd *cmd;
 
@@ -1691,12 +1681,6 @@
 	cmd = talloc_zero(lchan->ts->trx, struct sapi_cmd);
 	cmd->type = SAPI_CMD_SACCH_REL_MARKER;
 	queue_sapi_command(lchan, cmd);
-}
-
-static int lchan_deactivate_sacch(struct gsm_lchan *lchan)
-{
-	enqueue_sacch_rel_marker(lchan);
-	return 0;
 }
 
 /* callback from OML */
diff --git a/src/osmo-bts-sysmo/sysmobts_vty.c b/src/osmo-bts-sysmo/sysmobts_vty.c
index c829c49..8140090 100644
--- a/src/osmo-bts-sysmo/sysmobts_vty.c
+++ b/src/osmo-bts-sysmo/sysmobts_vty.c
@@ -42,15 +42,12 @@
 #include <osmo-bts/gsm_data.h>
 #include <osmo-bts/phy_link.h>
 #include <osmo-bts/logging.h>
+#include <osmo-bts/oml.h>
 #include <osmo-bts/vty.h>
 
 #include "femtobts.h"
 #include "l1_if.h"
 #include "utils.h"
-
-
-extern int lchan_activate(struct gsm_lchan *lchan);
-extern int lchan_deactivate(struct gsm_lchan *lchan);
 
 #define TRX_STR "Transceiver related commands\n" "TRX number\n"
 
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index d1dc34a..2b11cac 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -275,3 +275,9 @@
 int bts_model_adjst_ms_pwr(struct gsm_lchan *lchan) { return 0; }
 int bts_model_ts_disconnect(struct gsm_bts_trx_ts *ts) { return 0; }
 int bts_model_ts_connect(struct gsm_bts_trx_ts *ts, enum gsm_phys_chan_config as_pchan) { return 0; }
+void enqueue_rel_marker(struct gsm_lchan *lchan)
+{ return; }
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
+{ return; }
+int lchan_activate(struct gsm_lchan *lchan)
+{ return 0; }
diff --git a/tests/stubs.c b/tests/stubs.c
index c680db0..e406923 100644
--- a/tests/stubs.c
+++ b/tests/stubs.c
@@ -40,6 +40,12 @@
 int l1if_set_txpower(struct femtol1_hdl *fl1h, float tx_power)
 { return 0; }
 
+void enqueue_rel_marker(struct gsm_lchan *lchan)
+{ return; }
+void enqueue_sacch_rel_marker(struct gsm_lchan *lchan)
+{ return; }
+int lchan_activate(struct gsm_lchan *lchan)
+{ return 0; }
 
 int bts_model_adjst_ms_pwr(struct gsm_lchan *lchan)
 { return 0; }

-- 
To view, visit https://gerrit.osmocom.org/1046
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id0d3b19dbfaa16d1734321a07a6eb0355bfd77c9
Gerrit-PatchSet: 5
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>



More information about the gerrit-log mailing list