summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGeorge Joseph <george.joseph@fairview5.com>2014-10-14 20:46:31 +0000
committerGeorge Joseph <george.joseph@fairview5.com>2014-10-14 20:46:31 +0000
commit54a5b8798f530915d6e6a52d71fe1ac3a2633c5a (patch)
tree7887241a97e8ef4775d74443ace2de2c42655a56
parent2d07283dc5502f6521199aa4b6d1845a157afd4f (diff)
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/ ........ Merged revisions 425525 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/13@425526 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r--include/asterisk/config.h7
-rw-r--r--main/config.c4
-rw-r--r--tests/test_config.c29
3 files changed, 19 insertions, 21 deletions
diff --git a/include/asterisk/config.h b/include/asterisk/config.h
index 98cd71445..0919a58bc 100644
--- 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 7009e7d08..7e6a4d1c1 100644
--- 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 b1f55c782..be4bee3d2 100644
--- a/tests/test_config.c
+++ b/tests/test_config.c
@@ -395,26 +395,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);
@@ -429,7 +409,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;
}
@@ -448,6 +428,13 @@ AST_TEST_DEFINE(config_basic_ops)
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")) {
ast_test_status_update(test, "Should have returned test3 instead of %s\n", cat_name);
goto out;