Change the subscriber and database backend

gsm_subscriber is now refcounted, the db backend is leaking
a lot less, db_get_subscriber will allocate the subscr record
now, subscr_* will look up a subscriber in the list of currently
active subscribers and add an ref to this one.

The db test cases pass, more testing will be when next to the bts

7 files changed, 148 insertions, 72 deletions

diff --git a/include/openbsc/db.h b/include/openbsc/db.h
index df703e04e..bd30aabec 100644
--- a/include/openbsc/db.h
+++ b/include/openbsc/db.h
@@ -1,4 +1,5 @@
 /* (C) 2008 by Jan Luebbe <jluebbe@debian.org>
+ * (C) 2009 by Holger Hans Peter Freyther <zecke@selfish.org>
  * All Rights Reserved
  *
  * This program is free software; you can redistribute it and/or modify
@@ -24,14 +25,16 @@
 
 #include <openbsc/gsm_subscriber.h>
 
+/* one time initialisation */
 int db_init(const char *name);
 int db_prepare();
 int db_fini();
 
-struct gsm_subscriber* db_create_subscriber(char imsi[GSM_IMSI_LENGTH]);
-int db_get_subscriber(enum gsm_subscriber_field field, struct gsm_subscriber* subscriber);
-int db_set_subscriber(struct gsm_subscriber* subscriber);
+/* subscriber management */
+struct gsm_subscriber* db_create_subscriber(char *imsi);
+struct gsm_subscriber* db_get_subscriber(enum gsm_subscriber_field field, char *subscr);
+int db_sync_subscriber(struct gsm_subscriber* subscriber);
 int db_subscriber_alloc_tmsi(struct gsm_subscriber* subscriber);
-int db_subscriber_assoc_imei(struct gsm_subscriber* subscriber, char imei[GSM_IMEI_LENGTH]);
+int db_subscriber_assoc_imei(struct gsm_subscriber* subscriber, char *imei);
 
 #endif /* _DB_H */
diff --git a/include/openbsc/gsm_subscriber.h b/include/openbsc/gsm_subscriber.h
index 97c7665e3..90117d3ef 100644
--- a/include/openbsc/gsm_subscriber.h
+++ b/include/openbsc/gsm_subscriber.h
@@ -3,6 +3,7 @@
 
 #include <sys/types.h>
 #include "gsm_data.h"
+#include "linuxlist.h"
 
 #define GSM_IMEI_LENGTH 17
 #define GSM_IMSI_LENGTH 17
@@ -18,6 +19,10 @@ struct gsm_subscriber {
 	char name[GSM_NAME_LENGTH];
 	char extension[GSM_EXTENSION_LENGTH];
 	int authorized;
+
+	/* for internal management */ 
+	int use_count;
+	struct llist_head entry;
 };
 
 enum gsm_subscriber_field {
@@ -25,4 +30,8 @@ enum gsm_subscriber_field {
 	GSM_SUBSCRIBER_TMSI,
 };
 
+struct gsm_subscriber *subscr_alloc();
+struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr);
+struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr);
+
 #endif /* _GSM_SUBSCR_H */
diff --git a/src/chan_alloc.c b/src/chan_alloc.c
index 98588b18f..439a8da56 100644
--- a/src/chan_alloc.c
+++ b/src/chan_alloc.c
@@ -188,7 +188,10 @@ struct gsm_lchan *lchan_alloc(struct gsm_bts *bts, enum gsm_chan_t type)
 void lchan_free(struct gsm_lchan *lchan)
 {
 	lchan->type = GSM_LCHAN_NONE;
-	lchan->subscr = 0;
+	if (lchan->subscr) {
+		subscr_put(lchan->subscr);
+		lchan->subscr = 0;
+	}
 
 	/* stop the timer */
 	del_timer(&lchan->release_timer);
diff --git a/src/db.c b/src/db.c
index e8c6ac941..8ee213224 100644
--- a/src/db.c
+++ b/src/db.c
@@ -1,4 +1,6 @@
+/* Simple HLR/VLR database backend using dbi */
 /* (C) 2008 by Jan Luebbe <jluebbe@debian.org>
+ * (C) 2009 by Holger Hans Peter Freyther <zecke@selfish.org>
  * All Rights Reserved
  *
  * This program is free software; you can redistribute it and/or modify
@@ -29,7 +31,7 @@ static char *db_basename = NULL;
 static char *db_dirname = NULL;
 dbi_conn conn;
 
-void db__error_func(dbi_conn conn, void* data) {
+void db_error_func(dbi_conn conn, void* data) {
 	const char* msg;
 	dbi_conn_error(conn, &msg);
 	printf("DBI: %s\n", msg);
@@ -43,7 +45,7 @@ int db_init(const char *name) {
 		return 1;
 	}
 
-	dbi_conn_error_handler( conn, db__error_func, NULL );
+	dbi_conn_error_handler( conn, db_error_func, NULL );
 
 	/* MySQL
 	dbi_conn_set_option(conn, "host", "localhost");
@@ -54,8 +56,8 @@ int db_init(const char *name) {
 	*/
 
 	/* SqLite 3 */
-	char *db_basename = strdup(name);
-	char *db_dirname = strdup(name);
+	db_basename = strdup(name);
+	db_dirname = strdup(name);
 	dbi_conn_set_option(conn, "sqlite3_dbdir", dirname(db_dirname));
 	dbi_conn_set_option(conn, "dbname", basename(db_basename));
 
@@ -154,15 +156,13 @@ int db_fini() {
 	return 0;
 }
 
-struct gsm_subscriber* db_create_subscriber(char imsi[GSM_IMSI_LENGTH]) {
+struct gsm_subscriber* db_create_subscriber(char *imsi) {
 	dbi_result result;
-	struct gsm_subscriber* subscriber;
-	subscriber = malloc(sizeof(*subscriber));
-	if (!subscriber)
-		return NULL;
-	memset(subscriber, 0, sizeof(*subscriber));
-	strncpy(subscriber->imsi, imsi, GSM_IMSI_LENGTH-1);
-	if (!db_get_subscriber(GSM_SUBSCRIBER_IMSI, subscriber)) {
+	struct gsm_subscriber* subscr;
+
+	/* Is this subscriber known in the db? */
+	subscr = db_get_subscriber(GSM_SUBSCRIBER_IMSI, imsi); 
+	if (subscr) {
 		result = dbi_conn_queryf(conn,
                          "UPDATE Subscriber set updated = datetime('now') "
                          "WHERE imsi = %s " , imsi);
@@ -171,8 +171,12 @@ struct gsm_subscriber* db_create_subscriber(char imsi[GSM_IMSI_LENGTH]) {
 		} else {
 			dbi_result_free(result);
 		}
-		return subscriber;
+		return subscr;
 	}
+
+	subscr = subscr_alloc();
+	if (!subscr)
+		return NULL;
 	result = dbi_conn_queryf(conn,
 		"INSERT INTO Subscriber "
 		"(imsi, created, updated) "
@@ -183,76 +187,81 @@ struct gsm_subscriber* db_create_subscriber(char imsi[GSM_IMSI_LENGTH]) {
 	if (result==NULL) {
 		printf("DB: Failed to create Subscriber by IMSI.\n");
 	}
-	subscriber->id = dbi_conn_sequence_last(conn, NULL);
+	subscr->id = dbi_conn_sequence_last(conn, NULL);
+	strncpy(subscr->imsi, imsi, GSM_IMSI_LENGTH-1);
 	dbi_result_free(result);
-	printf("DB: New Subscriber: ID %llu, IMSI %s\n", subscriber->id, subscriber->imsi);
-	return subscriber;
+	printf("DB: New Subscriber: ID %llu, IMSI %s\n", subscr->id, subscr->imsi);
+	return subscr;
 }
 
-int db_get_subscriber(enum gsm_subscriber_field field, struct gsm_subscriber* subscriber) {
+struct gsm_subscriber *db_get_subscriber(enum gsm_subscriber_field field, char *id) {
 	dbi_result result;
 	const char *string;
 	char *quoted;
+	struct gsm_subscriber *subscr;
 
 	switch (field) {
 	case GSM_SUBSCRIBER_IMSI:
-		dbi_conn_quote_string_copy(conn, subscriber->imsi, &quoted);
+		dbi_conn_quote_string_copy(conn, id, &quoted);
 		result = dbi_conn_queryf(conn,
 			"SELECT * FROM Subscriber "
 			"WHERE imsi = %s ",
 			quoted
 		);
+		free(quoted);
 		break;
 	case GSM_SUBSCRIBER_TMSI:
-		dbi_conn_quote_string_copy(conn, subscriber->tmsi, &quoted);
+		dbi_conn_quote_string_copy(conn, id, &quoted);
 		result = dbi_conn_queryf(conn,
 			"SELECT * FROM Subscriber "
 			"WHERE tmsi = %s ",
 			quoted
 		);
+		free(quoted);
 		break;
 	default:
 		printf("DB: Unknown query selector for Subscriber.\n");
-		return 1;
+		return NULL;
 	}
 	if (result==NULL) {
 		printf("DB: Failed to query Subscriber.\n");
-		return 1;
+		return NULL;
 	}
 	if (!dbi_result_next_row(result)) {
 		printf("DB: Failed to find the Subscriber.\n");
 		dbi_result_free(result);
-		return 1;
+		return NULL;
 	}
-	memset(subscriber, 0, sizeof(*subscriber));
-	subscriber->id = dbi_result_get_ulonglong(result, "id");
+
+	subscr = subscr_alloc();
+	subscr->id = dbi_result_get_ulonglong(result, "id");
 	string = dbi_result_get_string(result, "imsi");
 	if (string)
-		strncpy(subscriber->imsi, string, GSM_IMSI_LENGTH);
+		strncpy(subscr->imsi, string, GSM_IMSI_LENGTH);
 
 	string = dbi_result_get_string(result, "tmsi");
 	if (string)
-		strncpy(subscriber->tmsi, string, GSM_TMSI_LENGTH);
+		strncpy(subscr->tmsi, string, GSM_TMSI_LENGTH);
 
 	string = dbi_result_get_string(result, "name");
 	if (string)
-		strncpy(subscriber->name, string, GSM_NAME_LENGTH);
+		strncpy(subscr->name, string, GSM_NAME_LENGTH);
 
 	string = dbi_result_get_string(result, "extension");
 	if (string)
-		strncpy(subscriber->extension, string, GSM_EXTENSION_LENGTH);
+		strncpy(subscr->extension, string, GSM_EXTENSION_LENGTH);
 
 	// FIXME handle extension
-	subscriber->lac = dbi_result_get_uint(result, "lac");
-	subscriber->authorized = dbi_result_get_uint(result, "authorized");
+	subscr->lac = dbi_result_get_uint(result, "lac");
+	subscr->authorized = dbi_result_get_uint(result, "authorized");
 	printf("DB: Found Subscriber: ID %llu, IMSI %s, NAME '%s', TMSI %s, LAC %hu, AUTH %u\n",
-		subscriber->id, subscriber->imsi, subscriber->name, subscriber->tmsi,
-		subscriber->lac, subscriber->authorized);
+		subscr->id, subscr->imsi, subscr->name, subscr->tmsi,
+		subscr->lac, subscr->authorized);
 	dbi_result_free(result);
-	return 0;
+	return subscr;
 }
 
-int db_set_subscriber(struct gsm_subscriber* subscriber) {
+int db_sync_subscriber(struct gsm_subscriber* subscriber) {
 	dbi_result result;
 	result = dbi_conn_queryf(conn,
 		"UPDATE Subscriber "
@@ -263,6 +272,7 @@ int db_set_subscriber(struct gsm_subscriber* subscriber) {
 		"WHERE imsi = %s ",
 		subscriber->tmsi, subscriber->lac, subscriber->authorized, subscriber->imsi
 	);
+
 	if (result==NULL) {
 		printf("DB: Failed to update Subscriber (by IMSI).\n");
 		return 1;
@@ -282,6 +292,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber* subscriber) {
 			"WHERE tmsi = %s ",
 			tmsi_quoted
 		);
+		free(tmsi_quoted);
 		if (result==NULL) {
 			printf("DB: Failed to query Subscriber while allocating new TMSI.\n");
 			return 1;
@@ -293,7 +304,7 @@ int db_subscriber_alloc_tmsi(struct gsm_subscriber* subscriber) {
 		if (!dbi_result_next_row(result)) {
 			dbi_result_free(result);
 			printf("DB: Allocated TMSI %s for IMSI %s.\n", subscriber->tmsi, subscriber->imsi);
-			return db_set_subscriber(subscriber);
+			return db_sync_subscriber(subscriber);
 		}
 		dbi_result_free(result);
 	}
diff --git a/src/gsm_subscriber.c b/src/gsm_subscriber.c
index fcf91ae51..6a25009f4 100644
--- a/src/gsm_subscriber.c
+++ b/src/gsm_subscriber.c
@@ -1,6 +1,7 @@
 /* Dummy implementation of a subscriber database, roghly HLR/VLR functionality */
 
 /* (C) 2008 by Harald Welte <laforge@gnumonks.org>
+ * (C) 2009 by Holger Hans Peter Freyther <zecke@selfish.org>
  *
  * All Rights Reserved
  *
@@ -28,6 +29,9 @@
 #include <openbsc/gsm_subscriber.h>
 #include <openbsc/db.h>
 
+
+LLIST_HEAD(active_subscribers);
+
 struct gsm_subscriber *subscr_alloc(void)
 {
 	struct gsm_subscriber *s;
@@ -37,46 +41,57 @@ struct gsm_subscriber *subscr_alloc(void)
 		return NULL;
 
 	memset(s, 0, sizeof(*s));
+	llist_add_tail(&s->entry, &active_subscribers);
+	s->use_count = 1;
 
 	return s;
 }
 
-void subscr_free(struct gsm_subscriber *subscr)
+static void subscr_free(struct gsm_subscriber *subscr)
 {
+	llist_del(&subscr->entry);
 	free(subscr);
 }
 
 struct gsm_subscriber *subscr_get_by_tmsi(char *tmsi)
 {
-	struct gsm_subscriber *subscr = subscr_alloc();
+	struct gsm_subscriber *subscr;
 
-	strncpy(subscr->tmsi, tmsi, sizeof(subscr->tmsi));
-	subscr->tmsi[sizeof(subscr->tmsi)-1] = '\0';
-
-	if (db_get_subscriber(GSM_SUBSCRIBER_TMSI, subscr) != 0) {
-		subscr_free(subscr);
-		subscr = NULL;
+	/* we might have a record in memory already */
+	llist_for_each_entry(subscr, &active_subscribers, entry) {
+		if (strcmp(subscr->tmsi, tmsi) == 0)
+			return subscr_get(subscr);
 	}
 
-	return subscr;
+	return db_get_subscriber(GSM_SUBSCRIBER_TMSI, tmsi);
 }
 
 struct gsm_subscriber *subscr_get_by_imsi(char *imsi)
 {
-	struct gsm_subscriber *subscr = subscr_alloc();
-
-	strncpy(subscr->imsi, imsi, sizeof(subscr->imsi));
-	subscr->imsi[sizeof(subscr->imsi)-1] = '\0';
+	struct gsm_subscriber *subscr;
 
-	if (db_get_subscriber(GSM_SUBSCRIBER_IMSI, subscr) != 0) {
-		subscr_free(subscr);
-		subscr = NULL;
+	llist_for_each_entry(subscr, &active_subscribers, entry) {
+		if (strcmp(subscr->imsi, imsi) == 0)
+			return subscr_get(subscr);
 	}
 
-	return subscr;
+	return db_get_subscriber(GSM_SUBSCRIBER_IMSI, imsi);
 }
 
 int subscr_update(struct gsm_subscriber *s, struct gsm_bts *bts)
 {
-	return db_set_subscriber(s);
+	return db_sync_subscriber(s);
+}
+
+struct gsm_subscriber *subscr_get(struct gsm_subscriber *subscr)
+{
+	subscr->use_count++;
+	return subscr;
+}
+
+struct gsm_subscriber *subscr_put(struct gsm_subscriber *subscr)
+{
+	if (--subscr->use_count <= 0)
+		subscr_free(subscr);
+	return NULL;
 }
diff --git a/tests/db/Makefile.am b/tests/db/Makefile.am
index 936263b7e..5c9c784c6 100644
--- a/tests/db/Makefile.am
+++ b/tests/db/Makefile.am
@@ -1,8 +1,8 @@
 INCLUDES = $(all_includes) -I$(top_srcdir)/include
-AM_CFLAGS=-Wall
+AM_CFLAGS=-Wall -ggdb3
 
 noinst_PROGRAMS = db_test
 
-db_test_SOURCES = db_test.c $(top_srcdir)/src/db.c
+db_test_SOURCES = db_test.c $(top_srcdir)/src/db.c $(top_srcdir)/src/gsm_subscriber.c
 db_test_LDADD = -ldl -ldbi
 
diff --git a/tests/db/db_test.c b/tests/db/db_test.c
index 760d7ab7a..a6632b7f9 100644
--- a/tests/db/db_test.c
+++ b/tests/db/db_test.c
@@ -1,4 +1,5 @@
 /* (C) 2008 by Jan Luebbe <jluebbe@debian.org>
+ * (C) 2009 by Holger Hans Peter Freyther <zecke@selfish.org>
  * All Rights Reserved
  *
  * This program is free software; you can redistribute it and/or modify
@@ -23,6 +24,30 @@
 #include <string.h>
 #include <malloc.h>
 
+#define COMPARE(original, copy) \
+	if (original->id != copy->id) \
+		fprintf(stderr, "Ids do not match in %s:%d %llu %llu\n", \
+			__FUNCTION__, __LINE__, original->id, copy->id); \
+	if (original->lac != copy->lac) \
+		fprintf(stderr, "LAC do not match in %s:%d %d %d\n", \
+			__FUNCTION__, __LINE__, original->lac, copy->lac); \
+	if (original->authorized != copy->authorized) \
+		fprintf(stderr, "Authorize do not match in %s:%d %d %d\n", \
+			__FUNCTION__, __LINE__, original->authorized, \
+			copy->authorized); \
+	if (strcmp(original->imsi, copy->imsi) != 0) \
+		fprintf(stderr, "IMSIs do not match in %s:%d '%s' '%s'\n", \
+			__FUNCTION__, __LINE__, original->imsi, copy->imsi); \
+	if (strcmp(original->tmsi, copy->tmsi) != 0) \
+		fprintf(stderr, "TMSIs do not match in %s:%d '%s' '%s'\n", \
+			__FUNCTION__, __LINE__, original->tmsi, copy->tmsi); \
+	if (strcmp(original->name, copy->name) != 0) \
+		fprintf(stderr, "names do not match in %s:%d '%s' '%s'\n", \
+			__FUNCTION__, __LINE__, original->name, copy->name); \
+	if (strcmp(original->extension, copy->extension) != 0) \
+		fprintf(stderr, "names do not match in %s:%d '%s' '%s'\n", \
+			__FUNCTION__, __LINE__, original->extension, copy->extension); \
+
 int main() {
 
 	if (db_init("hlr.sqlite3")) {
@@ -38,28 +63,38 @@ int main() {
 	printf("DB: Database prepared.\n");
 
 	struct gsm_subscriber *alice = NULL;
+	struct gsm_subscriber *alice_db;
 
-	alice = db_create_subscriber("3243245432345");
-	db_set_subscriber(alice);
-	db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice);
-	free(alice);
+	char *alice_imsi = "3243245432345";
+	alice = db_create_subscriber(alice_imsi);
+	db_sync_subscriber(alice);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice->imsi);
+	COMPARE(alice, alice_db);
+	subscr_put(alice_db);
+	subscr_put(alice);
 
-	alice = db_create_subscriber("3693245423445");
+	alice_imsi = "3693245423445";
+	alice = db_create_subscriber(alice_imsi);
 	db_subscriber_assoc_imei(alice, "1234567890");
 	db_subscriber_alloc_tmsi(alice);
 	alice->lac=42;
-	db_set_subscriber(alice);
-	db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice);
-	free(alice);
+	db_sync_subscriber(alice);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
+	COMPARE(alice, alice_db);
+	subscr_put(alice);
+	subscr_put(alice_db);
 
-	alice = db_create_subscriber("9993245423445");
+	alice_imsi = "9993245423445";
+	alice = db_create_subscriber(alice_imsi);
 	db_subscriber_alloc_tmsi(alice);
 	alice->lac=42;
-	db_set_subscriber(alice);
+	db_sync_subscriber(alice);
 	db_subscriber_assoc_imei(alice, "1234567890");
 	db_subscriber_assoc_imei(alice, "6543560920");
-	db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice);
-	free(alice);
+	alice_db = db_get_subscriber(GSM_SUBSCRIBER_IMSI, alice_imsi);
+	COMPARE(alice, alice_db);
+	subscr_put(alice);
+	subscr_put(alice_db);
 
 	db_fini();