From 12aa50d5a26aefa739da4f1e95c0e2921e704842 Mon Sep 17 00:00:00 2001 From: Holger Freyther Date: Thu, 1 Jan 2009 18:02:05 +0000 Subject: [PATCH] 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 --- include/openbsc/db.h | 11 +++-- include/openbsc/gsm_subscriber.h | 9 ++++ src/chan_alloc.c | 5 +- src/db.c | 81 ++++++++++++++++++-------------- src/gsm_subscriber.c | 51 +++++++++++++------- tests/db/Makefile.am | 4 +- tests/db/db_test.c | 59 ++++++++++++++++++----- 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 + * (C) 2009 by Holger Hans Peter Freyther * All Rights Reserved * * This program is free software; you can redistribute it and/or modify @@ -24,14 +25,16 @@ #include +/* 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 #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 + * (C) 2009 by Holger Hans Peter Freyther * 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, "ed); + dbi_conn_quote_string_copy(conn, id, "ed); 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, "ed); + dbi_conn_quote_string_copy(conn, id, "ed); 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 + * (C) 2009 by Holger Hans Peter Freyther * * All Rights Reserved * @@ -28,6 +29,9 @@ #include #include + +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(); + struct gsm_subscriber *subscr; - strncpy(subscr->imsi, imsi, sizeof(subscr->imsi)); - subscr->imsi[sizeof(subscr->imsi)-1] = '\0'; - - 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 + * (C) 2009 by Holger Hans Peter Freyther * All Rights Reserved * * This program is free software; you can redistribute it and/or modify @@ -23,6 +24,30 @@ #include #include +#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();