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/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/12@425713 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/include/asterisk/config.h b/include/asterisk/config.h
index 0919a58bcc070fff9a7e7d9bee5139996d58caab..2f604e0be35572b15fafca7ad6ff3577f18fb706 100644 (file)
--- a/include/asterisk/config.h
+++ b/include/asterisk/config.h
@@ -238,6 +238,11 @@ struct ast_category *ast_category_browse_filtered(struct ast_config *config,
  *
  * \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 @@ char *ast_category_browse(struct ast_config *config, const char *prev_name);
  *
  * \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);

diff --git a/main/config.c b/main/config.c
index 4524cf8f83a8c5e68456cc69a415bd65b982b0d9..83be41d95a21bd188ddce25aa2a94e387619b068 100644 (file)
--- a/main/config.c
+++ b/main/config.c
@@ -677,10 +677,29 @@ const char *ast_config_option(struct ast_config *cfg, const char *cat, const cha
        return tmp;
 }
 
-const char *ast_variable_retrieve(struct ast_config *config,
-       const char *category, const char *variable)
+const char *ast_variable_retrieve(struct ast_config *config, const char *category, const char *variable)
 {
-       return ast_variable_retrieve_filtered(config, category, variable, NULL);
+       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,
@@ -860,6 +879,12 @@ struct ast_category *ast_category_get(const struct ast_config *config,
 {
        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)) {
                        return cat;
@@ -1188,7 +1213,6 @@ struct ast_category *ast_category_browse_filtered(struct ast_config *config,
        }
 
        cat = next_available_category(prev, category_name, filter);
-       config->last_browse = cat;
 
        return cat;
 }

diff --git a/tests/test_config.c b/tests/test_config.c
index cd56d5c68fcd16212c774a77cff0ce551d47d2ac..03eeb138b01814e515b9ad618718769edbb259a3 100644 (file)
--- a/tests/test_config.c
+++ b/tests/test_config.c
@@ -459,7 +459,7 @@ AST_TEST_DEFINE(config_basic_ops)
        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,18 +468,73 @@ AST_TEST_DEFINE(config_basic_ops)
 
        /* 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;
+       }
+
        res = AST_TEST_PASS;
 
 out: