config: Fix SEGV in unit test with MALLOC_DEBUG

With MALLOC_DEBUG the /main/config config_basic_ops test was causing a
SEGV while doing an ast_category_delete in an ast_category_browse loop.
Apparently this never worked but was also never tested.  I removed the
test, added 2 notes to config.h indicating that it's not supported and
added a few lines of code to ast_category_delete to prevent the SEGV
should someone attempt it in the future.

Tested-by: George Joseph
Review: https://reviewboard.asterisk.org/r/4078/

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

diff --git a/include/asterisk/config.h b/include/asterisk/config.h
index 98cd71445cb181ee83bd6d67b40a90ff8ff08712..0919a58bcc070fff9a7e7d9bee5139996d58caab 100644 (file)
--- a/include/asterisk/config.h
+++ b/include/asterisk/config.h
@@ -257,6 +257,10 @@ 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);
@@ -794,6 +798,9 @@ int ast_category_insert(struct ast_config *config, struct ast_category *cat, con
  * \param category category to delete
  *
  * \return the category after the deleted one which could be NULL.
+ *
+ * \note It is not safe to call ast_category_delete while browsing with
+ * ast_category_browse.  It is safe with ast_category_browse_filtered.
  */
 struct ast_category *ast_category_delete(struct ast_config *cfg, struct ast_category *category);
 

diff --git a/main/config.c b/main/config.c
index f29f3435567178fc7b849b84cac149402d416e73..4524cf8f83a8c5e68456cc69a415bd65b982b0d9 100644 (file)
--- a/main/config.c
+++ b/main/config.c
@@ -1336,6 +1336,10 @@ struct ast_category *ast_category_delete(struct ast_config *config,
 
        prev = category->prev;
 
+       if (config->last_browse == category) {
+               config->last_browse = prev;
+       }
+
        ast_category_destroy(category);
 
        return prev;

diff --git a/tests/test_config.c b/tests/test_config.c
index 08b56507202f5aafe7b80f7993d4c92a96aec81b..cd56d5c68fcd16212c774a77cff0ce551d47d2ac 100644 (file)
--- a/tests/test_config.c
+++ b/tests/test_config.c
@@ -394,26 +394,6 @@ AST_TEST_DEFINE(config_basic_ops)
                }
        }
 
-       /* Now: test0 test1 test3 test4 test2 */
-       /* Test in-flight deletion using ast_category_browse */
-       /* Delete test1 and continue */
-       cat_name = NULL;
-       for(i = 0; i < 5; i++) {
-               if (i == 2) {  /* 2 was already deleted above */
-                       continue;
-               }
-               snprintf(temp, sizeof(temp), "test%d", i);
-               cat_name = ast_category_browse(cfg, cat_name);
-               cat = ast_category_get(cfg, cat_name, NULL);
-               if (strcmp(cat_name, temp)) {
-                       ast_test_status_update(test, "Should have returned %s instead of %s: %d\n", temp, cat_name, i);
-                       goto out;
-               }
-               if (i == 1) {
-                       ast_category_delete(cfg, cat);
-               }
-       }
-
        /* Now: test0 test3 test4 test2 */
        /* delete the head item */
        cat = ast_category_browse_filtered(cfg, NULL, NULL, NULL);
@@ -428,7 +408,7 @@ AST_TEST_DEFINE(config_basic_ops)
        /* make sure head got updated to the new first element */
        cat = ast_category_browse_filtered(cfg, NULL, NULL, NULL);
        cat_name = ast_category_get_name(cat);
-       if (strcmp(cat_name, "test3")) {
+       if (strcmp(cat_name, "test1")) {
                ast_test_status_update(test, "Should have returned test3 instead of %s\n", cat_name);
                goto out;
        }
@@ -445,6 +425,13 @@ AST_TEST_DEFINE(config_basic_ops)
 
        /* There should now only be 2 elements in the list */
        cat = NULL;
+       cat = ast_category_browse_filtered(cfg, NULL, cat, NULL);
+       cat_name = ast_category_get_name(cat);
+       if (strcmp(cat_name, "test1")) {
+               ast_test_status_update(test, "Should have returned test1 instead of %s\n", cat_name);
+               goto out;
+       }
+
        cat = ast_category_browse_filtered(cfg, NULL, cat, NULL);
        cat_name = ast_category_get_name(cat);
        if (strcmp(cat_name, "test3")) {