[asterisk-commits] twilson: trunk r322982 - in /trunk: ./ main/db.c

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Fri Jun 10 10:30:55 CDT 2011


Author: twilson
Date: Fri Jun 10 10:30:50 2011
New Revision: 322982

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=322982
Log:
Merged revisions 322981 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/1.8

........
  r322981 | twilson | 2011-06-10 08:29:00 -0700 (Fri, 10 Jun 2011) | 11 lines
  
  Avoid a DB1 infinite loop bug
  
  Explicity check the last entry in the DB and make sure that we don't iterate
  past it. Since there can be no duplicates, this just makes sure that we stop
  after matching the last key.
  
  This patch also refactors the code to get away from some code duplication. A
  previous patch added many astdb tests and this patch passed them.
  
  Review: https://reviewboard.asterisk.org/r/1259/
........

Modified:
    trunk/   (props changed)
    trunk/main/db.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: trunk/main/db.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/db.c?view=diff&rev=322982&r1=322981&r2=322982
==============================================================================
--- trunk/main/db.c (original)
+++ trunk/main/db.c Fri Jun 10 10:30:50 2011
@@ -100,9 +100,12 @@
 	</manager>
  ***/
 
+#define MAX_DB_FIELD 256
+
 static DB *astdb;
 AST_MUTEX_DEFINE_STATIC(dblock);
 static ast_cond_t dbcond;
+typedef int (*process_keys_cb)(DBT *key, DBT *value, const char *filter, void *data);
 
 static void db_sync(void);
 
@@ -143,15 +146,84 @@
 	return 0;
 }
 
+static const char *dbt_data2str(DBT *dbt)
+{
+	char *data = "";
+
+	if (dbt->size) {
+		data = dbt->data;
+		data[dbt->size - 1] = '\0';
+	}
+
+	return data;
+}
+
+static inline const char *dbt_data2str_full(DBT *dbt, const char *def)
+{
+	return S_OR(dbt_data2str(dbt), def);
+}
+
+static int process_db_keys(process_keys_cb cb, void *data, const char *filter, int sync)
+{
+	DBT key = { 0, }, value = { 0, }, last_key = { 0, };
+	int counter = 0;
+	int res, last = 0;
+	char last_key_s[MAX_DB_FIELD];
+
+	ast_mutex_lock(&dblock);
+	if (dbinit()) {
+		ast_mutex_unlock(&dblock);
+		return -1;
+	}
+
+	/* Somehow, the database can become corrupted such that astdb->seq will continue looping through
+	 * the database indefinitely. The pointer to last_key.data ends up getting re-used by the BDB lib
+	 * so this specifically queries for the last entry, makes a copy of the key, and then uses it as
+	 * a sentinel to avoid repeatedly looping over the list. */
+
+	if (astdb->seq(astdb, &last_key, &value, R_LAST)) {
+		/* Empty database */
+		ast_mutex_unlock(&dblock);
+		return 0;
+	}
+
+	memcpy(last_key_s, last_key.data, MIN(last_key.size - 1, sizeof(last_key_s)));
+	last_key_s[last_key.size - 1] = '\0';
+	for (res = astdb->seq(astdb, &key, &value, R_FIRST);
+			!res;
+			res = astdb->seq(astdb, &key, &value, R_NEXT)) {
+		/* The callback might delete the key, so we have to check it before calling */
+		last = !strcmp(dbt_data2str_full(&key, "<bad key>"), last_key_s);
+		counter += cb(&key, &value, filter, data);
+		if (last) {
+			break;
+		}
+	}
+
+	if (sync) {
+		db_sync();
+	}
+
+	ast_mutex_unlock(&dblock);
+
+	return counter;
+}
+
+static int db_deltree_cb(DBT *key, DBT *value, const char *filter, void *data)
+{
+	int res = 0;
+
+	if (keymatch(dbt_data2str_full(key, "<bad key>"), filter)) {
+		astdb->del(astdb, key, 0);
+		res = 1;
+	}
+	return res;
+}
+
 int ast_db_deltree(const char *family, const char *keytree)
 {
-	char prefix[256];
-	DBT key, data;
-	char *keys;
-	int res;
-	int pass;
-	int counter = 0;
-	
+	char prefix[MAX_DB_FIELD];
+
 	if (family) {
 		if (keytree) {
 			snprintf(prefix, sizeof(prefix), "/%s/%s", family, keytree);
@@ -163,36 +235,13 @@
 	} else {
 		prefix[0] = '\0';
 	}
-	
-	ast_mutex_lock(&dblock);
-	if (dbinit()) {
-		ast_mutex_unlock(&dblock);
-		return -1;
-	}
-	
-	memset(&key, 0, sizeof(key));
-	memset(&data, 0, sizeof(data));
-	pass = 0;
-	while (!(res = astdb->seq(astdb, &key, &data, pass++ ? R_NEXT : R_FIRST))) {
-		if (key.size) {
-			keys = key.data;
-			keys[key.size - 1] = '\0';
-		} else {
-			keys = "<bad key>";
-		}
-		if (keymatch(keys, prefix)) {
-			astdb->del(astdb, &key, 0);
-			counter++;
-		}
-	}
-	db_sync();
-	ast_mutex_unlock(&dblock);
-	return counter;
+
+	return process_db_keys(db_deltree_cb, NULL, prefix, 1);
 }
 
 int ast_db_put(const char *family, const char *keys, const char *value)
 {
-	char fullkey[256];
+	char fullkey[MAX_DB_FIELD];
 	DBT key, data;
 	int res, fullkeylen;
 
@@ -214,12 +263,13 @@
 	ast_mutex_unlock(&dblock);
 	if (res)
 		ast_log(LOG_WARNING, "Unable to put value '%s' for key '%s' in family '%s'\n", value, keys, family);
+
 	return res;
 }
 
 int ast_db_get(const char *family, const char *keys, char *value, int valuelen)
 {
-	char fullkey[256] = "";
+	char fullkey[MAX_DB_FIELD] = "";
 	DBT key, data;
 	int res, fullkeylen;
 
@@ -263,7 +313,7 @@
 
 int ast_db_del(const char *family, const char *keys)
 {
-	char fullkey[256];
+	char fullkey[MAX_DB_FIELD];
 	DBT key;
 	int res, fullkeylen;
 
@@ -319,7 +369,7 @@
 static char *handle_cli_database_get(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	int res;
-	char tmp[256];
+	char tmp[MAX_DB_FIELD];
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -402,13 +452,23 @@
 	return CLI_SUCCESS;
 }
 
+static int db_show_cb(DBT *key, DBT *value, const char *filter, void *data)
+{
+	struct ast_cli_args *a = data;
+	const char *key_s = dbt_data2str_full(key, "<bad key>");
+	const char *value_s = dbt_data2str_full(value, "<bad value>");
+
+	if (keymatch(key_s, filter)) {
+		ast_cli(a->fd, "%-50s: %-25s\n", key_s, value_s);
+		return 1;
+	}
+
+	return 0;
+}
+
 static char *handle_cli_database_show(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
-	char prefix[256];
-	DBT key, data;
-	char *keys, *values;
-	int res;
-	int pass;
+	char prefix[MAX_DB_FIELD];
 	int counter = 0;
 
 	switch (cmd) {
@@ -435,45 +495,33 @@
 	} else {
 		return CLI_SHOWUSAGE;
 	}
-	ast_mutex_lock(&dblock);
-	if (dbinit()) {
-		ast_mutex_unlock(&dblock);
+
+	if((counter = process_db_keys(db_show_cb, a, prefix, 0)) < 0) {
 		ast_cli(a->fd, "Database unavailable\n");
-		return CLI_SUCCESS;	
-	}
-	memset(&key, 0, sizeof(key));
-	memset(&data, 0, sizeof(data));
-	pass = 0;
-	while (!(res = astdb->seq(astdb, &key, &data, pass++ ? R_NEXT : R_FIRST))) {
-		if (key.size) {
-			keys = key.data;
-			keys[key.size - 1] = '\0';
-		} else {
-			keys = "<bad key>";
-		}
-		if (data.size) {
-			values = data.data;
-			values[data.size - 1]='\0';
-		} else {
-			values = "<bad value>";
-		}
-		if (keymatch(keys, prefix)) {
-			ast_cli(a->fd, "%-50s: %-25s\n", keys, values);
-			counter++;
-		}
-	}
-	ast_mutex_unlock(&dblock);
+		return CLI_SUCCESS;
+	}
+
 	ast_cli(a->fd, "%d results found.\n", counter);
-	return CLI_SUCCESS;	
+	return CLI_SUCCESS;
+}
+
+static int db_showkey_cb(DBT *key, DBT *value, const char *filter, void *data)
+{
+	struct ast_cli_args *a = data;
+	const char *key_s = dbt_data2str_full(key, "<bad key>");
+	const char *value_s = dbt_data2str_full(value, "<bad value>");
+
+	if (subkeymatch(key_s, filter)) {
+		ast_cli(a->fd, "%-50s: %-25s\n", key_s, value_s);
+		return 1;
+	}
+
+	return 0;
 }
 
 static char *handle_cli_database_showkey(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
-	char suffix[256];
-	DBT key, data;
-	char *keys, *values;
-	int res;
-	int pass;
+	char suffix[MAX_DB_FIELD];
 	int counter = 0;
 
 	switch (cmd) {
@@ -493,48 +541,40 @@
 	} else {
 		return CLI_SHOWUSAGE;
 	}
-	ast_mutex_lock(&dblock);
-	if (dbinit()) {
-		ast_mutex_unlock(&dblock);
+
+	if ((counter = process_db_keys(db_showkey_cb, a, suffix, 0)) < 0) {
 		ast_cli(a->fd, "Database unavailable\n");
-		return CLI_SUCCESS;	
-	}
-	memset(&key, 0, sizeof(key));
-	memset(&data, 0, sizeof(data));
-	pass = 0;
-	while (!(res = astdb->seq(astdb, &key, &data, pass++ ? R_NEXT : R_FIRST))) {
-		if (key.size) {
-			keys = key.data;
-			keys[key.size - 1] = '\0';
-		} else {
-			keys = "<bad key>";
-		}
-		if (data.size) {
-			values = data.data;
-			values[data.size - 1]='\0';
-		} else {
-			values = "<bad value>";
-		}
-		if (subkeymatch(keys, suffix)) {
-			ast_cli(a->fd, "%-50s: %-25s\n", keys, values);
-			counter++;
-		}
-	}
-	ast_mutex_unlock(&dblock);
+		return CLI_SUCCESS;
+	}
+
 	ast_cli(a->fd, "%d results found.\n", counter);
-	return CLI_SUCCESS;	
+	return CLI_SUCCESS;
+}
+
+static int db_gettree_cb(DBT *key, DBT *value, const char *filter, void *data)
+{
+	struct ast_db_entry **ret = data;
+	struct ast_db_entry *cur;
+	const char *key_s = dbt_data2str_full(key, "<bad key>");
+	const char *value_s = dbt_data2str_full(value, "<bad value>");
+	size_t key_slen = strlen(key_s) + 1, value_slen = strlen(value_s) + 1;
+
+	if (keymatch(key_s, filter) && (cur = ast_malloc(sizeof(*cur) + key_slen + value_slen))) {
+		cur->next = *ret;
+		cur->key = cur->data + value_slen;
+		strcpy(cur->data, value_s);
+		strcpy(cur->key, key_s);
+		*ret = cur;
+		return 1;
+	}
+
+	return 0;
 }
 
 struct ast_db_entry *ast_db_gettree(const char *family, const char *keytree)
 {
-	char prefix[256];
-	DBT key, data;
-	char *keys, *values;
-	int values_len;
-	int res;
-	int pass;
-	struct ast_db_entry *last = NULL;
-	struct ast_db_entry *cur, *ret=NULL;
+	char prefix[MAX_DB_FIELD];
+	struct ast_db_entry *ret = NULL;
 
 	if (!ast_strlen_zero(family)) {
 		if (!ast_strlen_zero(keytree)) {
@@ -547,44 +587,13 @@
 	} else {
 		prefix[0] = '\0';
 	}
-	ast_mutex_lock(&dblock);
-	if (dbinit()) {
-		ast_mutex_unlock(&dblock);
+
+	if (process_db_keys(db_gettree_cb, &ret, prefix, 0) < 0) {
 		ast_log(LOG_WARNING, "Database unavailable\n");
-		return NULL;	
-	}
-	memset(&key, 0, sizeof(key));
-	memset(&data, 0, sizeof(data));
-	pass = 0;
-	while (!(res = astdb->seq(astdb, &key, &data, pass++ ? R_NEXT : R_FIRST))) {
-		if (key.size) {
-			keys = key.data;
-			keys[key.size - 1] = '\0';
-		} else {
-			keys = "<bad key>";
-		}
-		if (data.size) {
-			values = data.data;
-			values[data.size - 1] = '\0';
-		} else {
-			values = "<bad value>";
-		}
-		values_len = strlen(values) + 1;
-		if (keymatch(keys, prefix) && (cur = ast_malloc(sizeof(*cur) + strlen(keys) + 1 + values_len))) {
-			cur->next = NULL;
-			cur->key = cur->data + values_len;
-			strcpy(cur->data, values);
-			strcpy(cur->key, keys);
-			if (last) {
-				last->next = cur;
-			} else {
-				ret = cur;
-			}
-			last = cur;
-		}
-	}
-	ast_mutex_unlock(&dblock);
-	return ret;	
+		return NULL;
+	}
+
+	return ret;
 }
 
 void ast_db_freetree(struct ast_db_entry *dbe)
@@ -637,7 +646,7 @@
 	char idText[256] = "";
 	const char *family = astman_get_header(m, "Family");
 	const char *key = astman_get_header(m, "Key");
-	char tmp[256];
+	char tmp[MAX_DB_FIELD];
 	int res;
 
 	if (ast_strlen_zero(family)) {




More information about the asterisk-commits mailing list