[svn-commits] murf: trunk r133299 - /trunk/main/pbx.c
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Wed Jul 23 17:03:48 CDT 2008
Author: murf
Date: Wed Jul 23 17:03:48 2008
New Revision: 133299
URL: http://svn.digium.com/view/asterisk?view=rev&rev=133299
Log:
(closes issue #13144)
Reported by: murf
Tested by: murf
For: J. Geis
The 'data' field in the ast_exten struct was being
'moved' from the current dialplan to the replacement
dialplan. This was not good, as the current dialplan
could have problems in the time between the change
and when the new dialplan is swapped in.
So, I modified the merge_and_delete code to strdup
the 'data' field (the args to the app call), and
then it's freed as normal.
I improved a few messages; I added code to limit
the number of calls to the context_merge_incls_swits_igps_other_registrars()
to one per context. I don't think having it called
multiple times per context was doing anything bad,
but it was inefficient.
I hope this fixes the problems Mr. Geiss was noting in
asterisk-users, see
http://lists.digium.com/pipermail/asterisk-users/2008-July/215634.html
Modified:
trunk/main/pbx.c
Modified: trunk/main/pbx.c
URL: http://svn.digium.com/view/asterisk/trunk/main/pbx.c?view=diff&rev=133299&r1=133298&r2=133299
==============================================================================
--- trunk/main/pbx.c (original)
+++ trunk/main/pbx.c Wed Jul 23 17:03:48 2008
@@ -345,6 +345,8 @@
{
const struct ast_context *ac = ah_a;
const struct ast_context *bc = ah_b;
+ if (!ac || !bc) /* safety valve, but it might prevent a crash you'd rather have happen */
+ return 1;
/* assume context names are registered in a string table! */
return strcmp(ac->name, bc->name);
}
@@ -788,7 +790,7 @@
for(c2=contexts;c2;c2=c2->next) {
c1 = find_context_locked(c2->name);
if (!c1) {
- ast_log(LOG_NOTICE,"Could not find the %s context in the hashtab\n", c2->name);
+ ast_log(LOG_NOTICE,"Called from: %s:%d: Could not find the %s context in the hashtab\n", file, line, c2->name);
check_contexts_trouble();
} else
ast_unlock_contexts();
@@ -4373,9 +4375,6 @@
previous_peer = peer;
}
}
-#ifdef CONTEXT_DEBUG
- check_contexts(__FILE__, __LINE__);
-#endif
if (!already_locked)
ast_unlock_context(con);
return found ? 0 : -1;
@@ -5697,14 +5696,17 @@
*local_contexts = tmp;
ast_hashtab_insert_safe(contexts_table, tmp); /*put this context into the tree */
ast_unlock_contexts();
+ ast_debug(1, "Registered context '%s'(%p) in table %p registrar: %s\n", tmp->name, tmp, contexts_table, registrar);
+ ast_verb(3, "Registered extension context '%s' (%p) in table %p; registrar: %s\n", tmp->name, tmp, contexts_table, registrar);
} else {
tmp->next = *local_contexts;
if (exttable)
ast_hashtab_insert_immediate(exttable, tmp); /*put this context into the tree */
+
*local_contexts = tmp;
- }
- ast_debug(1, "Registered context '%s'\n", tmp->name);
- ast_verb(3, "Registered extension context '%s'\n", tmp->name);
+ ast_debug(1, "Registered context '%s'(%p) in local table %p; registrar: %s\n", tmp->name, tmp, exttable, registrar);
+ ast_verb(3, "Registered extension context '%s' (%p) in local table %p; registrar: %s\n", tmp->name, tmp, exttable, registrar);
+ }
return tmp;
}
@@ -5761,11 +5763,13 @@
struct ast_hashtab_iter *exten_iter;
struct ast_hashtab_iter *prio_iter;
int insert_count = 0;
+ int first = 1;
/* We'll traverse all the extensions/prios, and see which are not registrar'd with
the current registrar, and copy them to the new context. If the new context does not
exist, we'll create it "on demand". If no items are in this context to copy, then we'll
only create the empty matching context if the old one meets the criteria */
+
if (context->root_table) {
exten_iter = ast_hashtab_start_traversal(context->root_table);
while ((exten_item=ast_hashtab_next(exten_iter))) {
@@ -5777,6 +5781,7 @@
prio_iter = ast_hashtab_start_traversal(exten_item->peer_table);
while ((prio_item=ast_hashtab_next(prio_iter))) {
int res1;
+ char *dupdstr;
if (new_exten_item) {
new_prio_item = ast_hashtab_lookup(new_exten_item->peer_table, prio_item);
@@ -5792,20 +5797,28 @@
}
/* copy in the includes, switches, and ignorepats */
- context_merge_incls_swits_igps_other_registrars(new, context, registrar);
+ if (first) { /* but, only need to do this once */
+ context_merge_incls_swits_igps_other_registrars(new, context, registrar);
+ first = 0;
+ }
+
if (!new) {
ast_log(LOG_ERROR,"Could not allocate a new context for %s in merge_and_delete! Danger!\n", context->name);
return; /* no sense continuing. */
}
/* we will not replace existing entries in the new context with stuff from the old context.
but, if this is because of some sort of registrar conflict, we ought to say something... */
+
+ dupdstr = ast_strdup(prio_item->data);
+
res1 = ast_add_extension2(new, 0, prio_item->exten, prio_item->priority, prio_item->label,
- prio_item->cidmatch, prio_item->app, prio_item->data, prio_item->datad, prio_item->registrar);
+ prio_item->cidmatch, prio_item->app, dupdstr, prio_item->datad, prio_item->registrar);
if (!res1 && new_exten_item && new_prio_item){
ast_verb(3,"Dropping old dialplan item %s/%s/%d [%s(%s)] (registrar=%s) due to conflict with new dialplan\n",
context->name, prio_item->exten, prio_item->priority, prio_item->app, (char*)prio_item->data, prio_item->registrar);
} else {
- prio_item->data = NULL; /* we pass the priority data from the old to the new */
+ /* we do NOT pass the priority data from the old to the new -- we pass a copy of it, so no changes to the current dialplan take place,
+ and no double frees take place, either! */
insert_count++;
}
}
@@ -5816,7 +5829,6 @@
if (!insert_count && !new && (strcmp(context->registrar, registrar) != 0 ||
(strcmp(context->registrar, registrar) == 0 && context->refcount > 1))) {
-
/* we could have given it the registrar of the other module who incremented the refcount,
but that's not available, so we give it the registrar we know about */
new = ast_context_find_or_create(extcontexts, exttable, context->name, context->registrar);
@@ -5939,7 +5951,7 @@
is now freely using the new stuff instead */
ast_hashtab_destroy(oldtable, NULL);
-
+
for (tmp = oldcontextslist; tmp; ) {
struct ast_context *next; /* next starting point */
next = tmp->next;
@@ -6633,8 +6645,12 @@
replacement? If so, replace, otherwise, bonk. */
if (!replace) {
ast_log(LOG_WARNING, "Unable to register extension '%s', priority %d in '%s', already in use\n", tmp->exten, tmp->priority, con->name);
- if (tmp->datad)
+ if (tmp->datad) {
tmp->datad(tmp->data);
+ /* if you free this, null it out */
+ tmp->data = NULL;
+ }
+
ast_free(tmp);
return -1;
}
@@ -6948,20 +6964,20 @@
}
if (option_debug) {
if (tmp->matchcid) {
- ast_debug(1, "Added extension '%s' priority %d (CID match '%s') to %s\n",
- tmp->exten, tmp->priority, tmp->cidmatch, con->name);
+ ast_debug(1, "Added extension '%s' priority %d (CID match '%s') to %s (%p)\n",
+ tmp->exten, tmp->priority, tmp->cidmatch, con->name, con);
} else {
- ast_debug(1, "Added extension '%s' priority %d to %s\n",
- tmp->exten, tmp->priority, con->name);
+ ast_debug(1, "Added extension '%s' priority %d to %s (%p)\n",
+ tmp->exten, tmp->priority, con->name, con);
}
}
if (tmp->matchcid) {
- ast_verb(3, "Added extension '%s' priority %d (CID match '%s') to %s\n",
- tmp->exten, tmp->priority, tmp->cidmatch, con->name);
+ ast_verb(3, "Added extension '%s' priority %d (CID match '%s') to %s (%p)\n",
+ tmp->exten, tmp->priority, tmp->cidmatch, con->name, con);
} else {
- ast_verb(3, "Added extension '%s' priority %d to %s\n",
- tmp->exten, tmp->priority, con->name);
+ ast_verb(3, "Added extension '%s' priority %d to %s (%p)\n",
+ tmp->exten, tmp->priority, con->name, con);
}
return 0;
@@ -7360,7 +7376,7 @@
struct ast_exten *e, *el, *en;
struct ast_ignorepat *ipi;
struct ast_context *tmp = con;
-
+
for (tmpi = tmp->includes; tmpi; ) { /* Free includes */
struct ast_include *tmpil = tmpi;
tmpi = tmpi->next;
More information about the svn-commits
mailing list