[svn-commits] gtjoseph: trunk r425527 - in /trunk: ./ include/asterisk/ main/ tests/

[SVN commits to the Digium repositories]

Author: gtjoseph
Date: Tue Oct 14 15:48:06 2014
New Revision: 425527

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=425527
Log:
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
........

Merged revisions 425526 from http://svn.asterisk.org/svn/asterisk/branches/13

Modified:
    trunk/   (props changed)
    trunk/include/asterisk/config.h
    trunk/main/config.c
    trunk/tests/test_config.c

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

Modified: trunk/include/asterisk/config.h
URL: http://svnview.digium.com/svn/asterisk/trunk/include/asterisk/config.h?view=diff&rev=425527&r1=425526&r2=425527
==============================================================================
--- trunk/include/asterisk/config.h (original)
+++ trunk/include/asterisk/config.h Tue Oct 14 15:48:06 2014
@@ -257,6 +257,10 @@
  *
  * \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 @@
  * \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);
 

Modified: trunk/main/config.c
URL: http://svnview.digium.com/svn/asterisk/trunk/main/config.c?view=diff&rev=425527&r1=425526&r2=425527
==============================================================================
--- trunk/main/config.c (original)
+++ trunk/main/config.c Tue Oct 14 15:48:06 2014
@@ -1335,6 +1335,10 @@
 	}
 
 	prev = category->prev;
+
+	if (config->last_browse == category) {
+		config->last_browse = prev;
+	}
 
 	ast_category_destroy(category);
 

Modified: trunk/tests/test_config.c
URL: http://svnview.digium.com/svn/asterisk/trunk/tests/test_config.c?view=diff&rev=425527&r1=425526&r2=425527
==============================================================================
--- trunk/tests/test_config.c (original)
+++ trunk/tests/test_config.c Tue Oct 14 15:48:06 2014
@@ -395,26 +395,6 @@
 		}
 	}
 
-	/* 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 @@
 	/* 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;
 	}
@@ -446,6 +426,13 @@
 
 	/* 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")) {