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

Max gerrit-no-reply at lists.osmocom.org
Wed Jan 4 16:57:49 UTC 2017


Review at  https://gerrit.osmocom.org/1532

Remove duplicated code

Functionality of osmo_gsm_timer is available in libosmocore as
osmo_timer hence there is no need for local copy.

Change-Id: Ic98d63e8c48fa9e0e17c90da7b0a0297d0e53d3b
---
M src/Makefile.am
M src/gprs_rlcmac.h
D src/gsm_timer.cpp
D src/gsm_timer.h
M src/pcu_main.cpp
M src/tbf.h
6 files changed, 9 insertions(+), 330 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/32/1532/1

diff --git a/src/Makefile.am b/src/Makefile.am
index 7cc239b..6e832d6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -45,7 +45,6 @@
 	gprs_rlcmac_ts_alloc.cpp \
 	gprs_ms.cpp \
 	gprs_ms_storage.cpp \
-	gsm_timer.cpp \
 	bitvector.cpp \
 	pcu_l1_if.cpp \
 	pcu_vty.c \
@@ -79,7 +78,6 @@
 	gprs_ms.h \
 	gprs_ms_storage.h \
 	pcu_l1_if.h \
-	gsm_timer.h \
 	bitvector.h \
 	pcu_vty.h \
 	pcu_vty_functions.h \
diff --git a/src/gprs_rlcmac.h b/src/gprs_rlcmac.h
index 589bd8f..f6aed03 100644
--- a/src/gprs_rlcmac.h
+++ b/src/gprs_rlcmac.h
@@ -24,7 +24,6 @@
 #ifdef __cplusplus
 #include <bitvector.h>
 #include <gsm_rlcmac.h>
-#include <gsm_timer.h>
 
 extern "C" {
 #include <osmocom/core/linuxlist.h>
diff --git a/src/gsm_timer.cpp b/src/gsm_timer.cpp
deleted file mode 100644
index d3c59cb..0000000
--- a/src/gsm_timer.cpp
+++ /dev/null
@@ -1,238 +0,0 @@
-/* gsm_timer.cpp
- *
- * Copyright (C) 2012 Ivan Klyuchnikov
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
- */
- 
-/* These store the amount of frame number that we wait until next timer expires. */
-static int nearest;
-static int *nearest_p;
-
-/*! \addtogroup gsm_timer
- *  @{
- */
-
-/*! \file gsm_timer.cpp
- */
-
-#include <assert.h>
-#include <string.h>
-#include <limits.h>
-#include <gsm_timer.h>
-#include <pcu_l1_if.h>
-#include <bts.h>
-
-
-static struct rb_root timer_root = RB_ROOT;
-
-/*
- * TODO: make this depend on the BTS. This means that
- * all time functions schedule based on the BTS they
- * are scheduled on.
- */
-static int get_current_fn()
-{
-	return BTS::main_bts()->current_frame_number();
-}
-
-static void __add_gsm_timer(struct osmo_gsm_timer_list *timer)
-{
-	struct rb_node **new_node = &(timer_root.rb_node);
-	struct rb_node *parent = NULL;
-
-	while (*new_node) {
-		struct osmo_gsm_timer_list *this_timer;
-
-		this_timer = container_of(*new_node, struct osmo_gsm_timer_list, node);
-
-		parent = *new_node;
-		if (timer->fn < this_timer->fn)
-			new_node = &((*new_node)->rb_left);
-		else
-			new_node = &((*new_node)->rb_right);
-		}
-
-		rb_link_node(&timer->node, parent, new_node);
-		rb_insert_color(&timer->node, &timer_root);
-}
-
-/*! \brief add a new timer to the timer management
- *  \param[in] timer the timer that should be added
- */
-void osmo_gsm_timer_add(struct osmo_gsm_timer_list *timer)
-{
-	osmo_gsm_timer_del(timer);
-	timer->active = 1;
-	INIT_LLIST_HEAD(&timer->list);
-	__add_gsm_timer(timer);
-}
-
-/*! \brief schedule a gsm timer at a given future relative time
- *  \param[in] timer the to-be-added timer
- *  \param[in] number of frames from now
- *
- * This function can be used to (re-)schedule a given timer at a
- * specified number of frames in the future.  It will
- * internally add it to the timer management data structures, thus
- * osmo_timer_add() is automatically called.
- */
-void
-osmo_gsm_timer_schedule(struct osmo_gsm_timer_list *timer, int fn)
-{
-	int current_fn;
-
-	current_fn = get_current_fn();
-	timer->fn = current_fn + fn;
-	osmo_gsm_timer_add(timer);
-}
-
-/*! \brief delete a gsm timer from timer management
- *  \param[in] timer the to-be-deleted timer
- *
- * This function can be used to delete a previously added/scheduled
- * timer from the timer management code.
- */
-void osmo_gsm_timer_del(struct osmo_gsm_timer_list *timer)
-{
-	if (timer->active) {
-		timer->active = 0;
-		rb_erase(&timer->node, &timer_root);
-		/* make sure this is not already scheduled for removal. */
-		if (!llist_empty(&timer->list))
-			llist_del_init(&timer->list);
-	}
-}
-
-/*! \brief check if given timer is still pending
- *  \param[in] timer the to-be-checked timer
- *  \return 1 if pending, 0 otherwise
- *
- * This function can be used to determine whether a given timer
- * has alredy expired (returns 0) or is still pending (returns 1)
- */
-int osmo_gsm_timer_pending(struct osmo_gsm_timer_list *timer)
-{
-	return timer->active;
-}
-
-/*
- * if we have a nearest frame number return the delta between the current
- * FN and the FN of the nearest timer.
- * If the nearest timer timed out return NULL and then we will
- * dispatch everything after the select
- */
-int *osmo_gsm_timers_nearest(void)
-{
-	/* nearest_p is exactly what we need already: NULL if nothing is
-	 * waiting, {0,0} if we must dispatch immediately, and the correct
-	 * delay if we need to wait */
-	return nearest_p;
-}
-
-static void update_nearest(int *cand, int *current)
-{
-	if (*cand != LONG_MAX) {
-		if (*cand > *current)
-			nearest = *cand - *current;
-		else {
-			/* loop again inmediately */
-			nearest = 0;
-		}
-		nearest_p = &nearest;
-	} else {
-		nearest_p = NULL;
-	}
-}
-
-/*
- * Find the nearest FN and update s_nearest_time
- */
-void osmo_gsm_timers_prepare(void)
-{
-	struct rb_node *node;
-	int current_fn;
-
-	current_fn = get_current_fn();
-
-	node = rb_first(&timer_root);
-	if (node) {
-		struct osmo_gsm_timer_list *this_timer;
-		this_timer = container_of(node, struct osmo_gsm_timer_list, node);
-		update_nearest(&this_timer->fn, &current_fn);
-	} else {
-		nearest_p = NULL;
-	}
-}
-
-/*
- * fire all timers... and remove them
- */
-int osmo_gsm_timers_update(void)
-{
-	int current_fn;
-	struct rb_node *node;
-	struct llist_head timer_eviction_list;
-	struct osmo_gsm_timer_list *this_timer;
-	int work = 0;
-
-	current_fn = get_current_fn();
-
-	INIT_LLIST_HEAD(&timer_eviction_list);
-	for (node = rb_first(&timer_root); node; node = rb_next(node)) {
-		this_timer = container_of(node, struct osmo_gsm_timer_list, node);
-
-		if (this_timer->fn > current_fn)
-			break;
-
-		llist_add(&this_timer->list, &timer_eviction_list);
-	}
-
-	/*
-	 * The callbacks might mess with our list and in this case
-	 * even llist_for_each_entry_safe is not safe to use. To allow
-	 * osmo_gsm_timer_del to be called from within the callback we need
-	 * to restart the iteration for each element scheduled for removal.
-	 *
-	 * The problematic scenario is the following: Given two timers A
-	 * and B that have expired at the same time. Thus, they are both
-	 * in the eviction list in this order: A, then B. If we remove
-	 * timer B from the A's callback, we continue with B in the next
-	 * iteration step, leading to an access-after-release.
-	 */
-restart:
-	llist_for_each_entry(this_timer, &timer_eviction_list, list) {
-		osmo_gsm_timer_del(this_timer);
-		this_timer->cb(this_timer->data);
-		work = 1;
-		goto restart;
-	}
-
-	return work;
-}
-
-int osmo_gsm_timers_check(void)
-{
-	struct rb_node *node;
-	int i = 0;
-
-	for (node = rb_first(&timer_root); node; node = rb_next(node)) {
-		i++;
-	}
-	return i;
-}
-
-/*! }@ */
-
diff --git a/src/gsm_timer.h b/src/gsm_timer.h
deleted file mode 100644
index fc42caf..0000000
--- a/src/gsm_timer.h
+++ /dev/null
@@ -1,84 +0,0 @@
-/* gsm_timer.h
- *
- * Copyright (C) 2012 Ivan Klyuchnikov
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * as published by the Free Software Foundation; either version 2
- * of the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
- */
-
-/*! \defgroup timer GSM timers
- *  @{
- */
-
-/*! \file gsm_timer.h
- *  \brief GSM timer handling routines
- */
-#ifndef GSM_TIMER_H
-#define GSM_TIMER_H
-
-extern "C" {
-#include <osmocom/core/linuxlist.h>
-#include <osmocom/core/linuxrbtree.h>
-}
-/**
- * Timer management:
- *      - Create a struct osmo_gsm_timer_list
- *      - Fill out timeout and use add_gsm_timer or
- *        use schedule_gsm_timer to schedule a timer in
- *        x frames from now...
- *      - Use del_gsm_timer to remove the timer
- *
- *  Internally:
- *      - We hook into select.c to give a frame number of the
- *        nearest timer. On already passed timers we give
- *        it a 0 to immediately fire after the select.
- *      - update_gsm_timers will call the callbacks and remove
- *        the timers.
- *
- */
-/*! \brief A structure representing a single instance of a gsm timer */
-struct osmo_gsm_timer_list {
-	struct rb_node node;	  /*!< \brief rb-tree node header */
-	struct llist_head list;   /*!< \brief internal list header */
-	int fn;                   /*!< \brief expiration frame number */
-	unsigned int active  : 1; /*!< \brief is it active? */
-
-	void (*cb)(void*);	  /*!< \brief call-back called at timeout */
-	void *data;		  /*!< \brief user data for callback */
-};
-
-/**
- * timer management
- */
-
-void osmo_gsm_timer_add(struct osmo_gsm_timer_list *timer);
-
-void osmo_gsm_timer_schedule(struct osmo_gsm_timer_list *timer, int fn);
-
-void osmo_gsm_timer_del(struct osmo_gsm_timer_list *timer);
-
-int osmo_gsm_timer_pending(struct osmo_gsm_timer_list *timer);
-
-
-/*
- * internal timer list management
- */
-int *osmo_gsm_timers_nearest(void);
-void osmo_gsm_timers_prepare(void);
-int osmo_gsm_timers_update(void);
-int osmo_gsm_timers_check(void);
-
-/*! }@ */
-
-#endif // GSM_TIMER_H
diff --git a/src/pcu_main.cpp b/src/pcu_main.cpp
index 0a0362b..c19a173 100644
--- a/src/pcu_main.cpp
+++ b/src/pcu_main.cpp
@@ -21,7 +21,6 @@
 #include <arpa/inet.h>
 #include <pcu_l1_if.h>
 #include <gprs_rlcmac.h>
-#include <gsm_timer.h>
 #include <gprs_debug.h>
 #include <unistd.h>
 #include <getopt.h>
@@ -34,6 +33,7 @@
 #include <osmocom/vty/logging.h>
 #include <osmocom/vty/ports.h>
 #include <osmocom/core/stats.h>
+#include <osmocom/core/timer.h>
 #include <osmocom/core/gsmtap.h>
 #include <osmocom/core/gsmtap_util.h>
 }
@@ -288,9 +288,9 @@
 	}
 
 	while (!quit) {
-		osmo_gsm_timers_check();
-		osmo_gsm_timers_prepare();
-		osmo_gsm_timers_update();
+		osmo_timers_check();
+		osmo_timers_prepare();
+		osmo_timers_update();
 
 		osmo_select_main(0);
 	}
diff --git a/src/tbf.h b/src/tbf.h
index ad1ece2..7aa78c5 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -20,6 +20,10 @@
 
 #ifdef __cplusplus
 
+extern "C" {
+#include <osmocom/core/timer.h>
+}
+
 #include "gprs_rlcmac.h"
 #include "llc.h"
 #include "rlc.h"
@@ -234,7 +238,7 @@
 	unsigned int T; /* Txxxx number */
 	unsigned int num_T_exp; /* number of consecutive T expirations */
 	
-	struct osmo_gsm_timer_list	gsm_timer;
+	struct osmo_timer_list	gsm_timer;
 	unsigned int fT; /* fTxxxx number */
 	unsigned int num_fT_exp; /* number of consecutive fT expirations */
 

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic98d63e8c48fa9e0e17c90da7b0a0297d0e53d3b
Gerrit-PatchSet: 1
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>


More information about the gerrit-log mailing list