[asterisk-commits] gtjoseph: branch 12 r425713 - in /branches/12: include/asterisk/ main/ tests/

[SVN commits to the Asterisk project]

Author: gtjoseph
Date: Thu Oct 16 12:29:58 2014
New Revision: 425713

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=425713
Log:
config: Fix inf loop using ast_category_browse and ast_variable_retrieve

Fix infinite loop when calling ast_variable_retrieve inside an
ast_category_browse loop when there is more than 1 category with
the same name.

Tested-by: George Joseph

Review: https://reviewboard.asterisk.org/r/4089/



Modified:
    branches/12/include/asterisk/config.h
    branches/12/main/config.c
    branches/12/tests/test_config.c

Modified: branches/12/include/asterisk/config.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/config.h?view=diff&rev=425713&r1=425712&r2=425713
==============================================================================
--- branches/12/include/asterisk/config.h (original)
+++ branches/12/include/asterisk/config.h Thu Oct 16 12:29:58 2014
@@ -238,6 +238,11 @@
  *
  * \retval a category name on success
  * \retval NULL on failure/no-more-categories
+ *
+ * \note ast_category_browse maintains internal state.  Therefore is not thread
+ * safe, cannot be called recursively, and it is not safe to add or remove
+ * categories while browsing.
+ * ast_category_browse_filtered does not have these restrictions.
  */
 char *ast_category_browse(struct ast_config *config, const char *prev_name);
 
@@ -257,10 +262,6 @@
  *
  * \retval ast_variable list on success
  * \retval NULL on failure
- *
- * \note ast_category_browse is not thread safe and it is not safe to call
- * ast_category_delete while browsing using ast_category_browse.
- * ast_category_browse_filtered does not have these restrictions.
  */
 struct ast_variable *ast_variable_browse_filtered(const struct ast_config *config,
 	const char *category_name, const char *filter);

Modified: branches/12/main/config.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/main/config.c?view=diff&rev=425713&r1=425712&r2=425713
==============================================================================
--- branches/12/main/config.c (original)
+++ branches/12/main/config.c Thu Oct 16 12:29:58 2014
@@ -677,10 +677,29 @@
 	return tmp;
 }
 
-const char *ast_variable_retrieve(struct ast_config *config,
-	const char *category, const char *variable)
-{
-	return ast_variable_retrieve_filtered(config, category, variable, NULL);
+const char *ast_variable_retrieve(struct ast_config *config, const char *category, const char *variable)
+{
+	struct ast_variable *v;
+
+	if (category) {
+		for (v = ast_variable_browse(config, category); v; v = v->next) {
+			if (!strcasecmp(variable, v->name)) {
+				return v->value;
+			}
+		}
+	} else {
+		struct ast_category *cat;
+
+		for (cat = config->root; cat; cat = cat->next) {
+			for (v = cat->root; v; v = v->next) {
+				if (!strcasecmp(variable, v->name)) {
+					return v->value;
+				}
+			}
+		}
+	}
+
+	return NULL;
 }
 
 const char *ast_variable_retrieve_filtered(struct ast_config *config,
@@ -859,6 +878,12 @@
 	const char *category_name, const char *filter)
 {
 	struct ast_category *cat;
+
+	for (cat = config->root; cat; cat = cat->next) {
+		if (cat->name == category_name && does_category_match(cat, category_name, filter)) {
+			return cat;
+		}
+	}
 
 	for (cat = config->root; cat; cat = cat->next) {
 		if (does_category_match(cat, category_name, filter)) {
@@ -1188,7 +1213,6 @@
 	}
 
 	cat = next_available_category(prev, category_name, filter);
-	config->last_browse = cat;
 
 	return cat;
 }

Modified: branches/12/tests/test_config.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/tests/test_config.c?view=diff&rev=425713&r1=425712&r2=425713
==============================================================================
--- branches/12/tests/test_config.c (original)
+++ branches/12/tests/test_config.c Thu Oct 16 12:29:58 2014
@@ -459,7 +459,7 @@
 	cat = ast_category_browse_filtered(cfg, NULL, NULL, NULL);
 	cat = ast_category_browse_filtered(cfg, NULL, cat, NULL);
 	cat_name = ast_category_get_name(cat);
-	var = ast_variable_new("aaa", "bbb", "dummy");
+	var = ast_variable_new("aaa", "bbb0", "dummy");
 	if (!var) {
 		ast_test_status_update(test, "Couldn't allocate variable.\n");
 		goto out;
@@ -468,15 +468,70 @@
 
 	/* Make sure we can retrieve with specific category name */
 	var_value = ast_variable_retrieve(cfg, cat_name, "aaa");
-	if (!var_value || strcmp(var_value, "bbb")) {
+	if (!var_value || strcmp(var_value, "bbb0")) {
 		ast_test_status_update(test, "Variable not found or wrong value.\n");
 		goto out;
 	}
 
 	/* Make sure we can retrieve with NULL category name */
 	var_value = ast_variable_retrieve(cfg, NULL, "aaa");
-	if (!var_value || strcmp(var_value, "bbb")) {
+	if (!var_value || strcmp(var_value, "bbb0")) {
 		ast_test_status_update(test, "Variable not found or wrong value.\n");
+		goto out;
+	}
+
+	/* Now test variable retrieve inside a browse loop
+	 * with multiple categories of the same name
+	 */
+	cat = ast_category_new("test3", "dummy", -1);
+	if (!cat) {
+		ast_test_status_update(test, "Couldn't allocate category.\n");
+		goto out;
+	}
+	var = ast_variable_new("aaa", "bbb1", "dummy");
+	if (!var) {
+		ast_test_status_update(test, "Couldn't allocate variable.\n");
+		goto out;
+	}
+	ast_variable_append(cat, var);
+	ast_category_append(cfg, cat);
+
+	cat = ast_category_new("test3", "dummy", -1);
+	if (!cat) {
+		ast_test_status_update(test, "Couldn't allocate category.\n");
+		goto out;
+	}
+	var = ast_variable_new("aaa", "bbb2", "dummy");
+	if (!var) {
+		ast_test_status_update(test, "Couldn't allocate variable.\n");
+		goto out;
+	}
+	ast_variable_append(cat, var);
+	ast_category_append(cfg, cat);
+
+	cat_name = NULL;
+	i = 0;
+	while ((cat_name = ast_category_browse(cfg, cat_name))) {
+		if (!strcmp(cat_name, "test3")) {
+			snprintf(temp, sizeof(temp), "bbb%d", i);
+
+			var_value = ast_variable_retrieve(cfg, cat_name, "aaa");
+			if (!var_value || strcmp(var_value, temp)) {
+				ast_test_status_update(test, "Variable not found or wrong value %s.\n", var_value);
+				goto out;
+			}
+
+			var = ast_variable_browse(cfg, cat_name);
+			if (!var->value || strcmp(var->value, temp)) {
+				ast_test_status_update(test, "Variable not found or wrong value %s.\n", var->value);
+				goto out;
+			}
+
+			i++;
+		}
+	}
+	if (i != 3) {
+		ast_test_status_update(test, "There should have been 3 matches instead of %d.\n", i);
 		goto out;
 	}