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")) { -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
