Submitter:	Mark Michelson
Branch:	/trunk
Bugs:	14615
Depends On:
Blocks:

Reviewers
Groups:	asterisk-dev
People:
Repository:	Asterisk

Description:

When looking into issue 14615, I found a variety of problems which I have addressed in my npm_fixes branch. Here is a summary of the fixes I made:

1. Create different trie entries for pattern and non-pattern matches which are otherwise identical. For example, if you had an extensions "NNN" and "_NNN" in your dialplan, then we need to create two separate trie entries for matching against.

2. Make sure to only apply pattern-matching logic if the extension we are matching against is a pattern extension. This is what bug 14615 is actually about. In that example, the reporter was attempting to Goto() the extension "ANSWER." The problem was that when the 'N' was encountered while looking at the pattern trie, it would not match the literal character 'N' that was being passed in. However, if the Goto() were modified to go to something like "A4SWER" it would go to the ANSWER extension. This behavior has been corrected.

3. Fixed a few places where the code could potentially crash due to dereferencing NULL pointers.

4. Changed the allocation scheme of a match_char to use the "allocation after the struct" method.

5. Did a bunch of coding guidelines-related cleanup.

6. Removed all blocks of code enclosed inside #ifdef NOTNOW...#endif. These areas were designed to use a more manual algorithm for looking up contexts than just finding them in a hashtab. The logic used in these blocks was off, and I can't think of any good reason why you would want to do this.

For anyone who wants to understand how the pattern-matching trie works before taking a crack at this review, you can find a large comment in main/pbx.c that Murf wrote that does an excellent job of explaining things. This comment block is located just below the function pbx_destroy().

Testing Done:

I tested the following dialplan, admittedly not the most thorough, but it gets the job done:

exten => 333,1,NoOp(Starting Test, exten 333)
exten => 333,n,Goto(ANSWER,1)
exten => 333,n,Hangup

exten => NNN,1,NoOp(Starting Test, exten NNN)
exten => NNN,n,Playback(tt-monkeys)
exten => NNN,n,Hangup

exten => _NNN,1,NoOp(Starting Test, exten _NNN)
exten => _NNN,n,Playback(tt-weasels)
exten => _NNN,n,Hangup

exten => ANSWER,1,NoOp(Test no answer)
exten => ANSWER,n,Playback(vm-goodbye)
exten => ANSWER,n,Hangup

exten => _1[2-9],1,NoOp(Starting Test, exten _1[2-9])
exten => _1[2-9],n,Playback(queue-thankyou)
exten => _1[2-9],n,Hangup

With this, I could test numeric extensions, string extensions, patterns with reserved letters (i.e. N, Z, X), and patterns with bracketed expressions. In all cases, the pattern I expected to match was matched. The reporter on issue 14615 has also given the branch a test and says that things work for him in his dialplan now.

Diff revision 1 (Latest)

Files Changed:

/trunk/main/pbx.c:

     /*! \brief match_char: forms a syntax tree for quick matching of extension patterns */
     struct match_char
+    {
     	int is_pattern; /* the pattern started with '_' */
     	int deleted;    /* if this is set, then... don't return it */
-    	char *x;       /* the pattern itself-- matches a single char */
     	int specificity; /* simply the strlen of x, or 10 for X, 9 for Z, and 8 for N; and '.' and '!' will add 11 ? */
     	struct match_char *alt_char;
     	struct match_char *next_char;
     	struct ast_exten *exten; /* attached to last char of a pattern for exten */
     	char x[1];       /* the pattern itself-- matches a single char */
     };
     struct scoreboard  /* make sure all fields are 0 before calling new_find_extension */
+    {
     	int total_specificity;
     static int pbx_builtin_importvar(struct ast_channel *, void *);
     static void set_ext_pri(struct ast_channel *c, const char *exten, int pri);
     static void new_find_extension(const char *str, struct scoreboard *score,
     		struct match_char *tree, int length, int spec, const char *callerid,
     		const char *label, enum ext_match_t action);
-    static struct match_char *already_in_tree(struct match_char *current, char *pat);
+    static struct match_char *already_in_tree(struct match_char *current, char *pat, int is_pattern);
     static struct match_char *add_exten_to_pattern_tree(struct ast_context *con,
     		struct ast_exten *e1, int findonly);
     static struct match_char *add_pattern_node(struct ast_context *con,
     		struct match_char *current, char *pattern, int is_pattern,
     		int already, int specificity, struct match_char **parent);
     	char extenstr[40];
     	struct ast_str *my_prefix = ast_str_alloca(1024);
     	extenstr[0] = '\0';
-    	if (node && node->exten && node->exten)
+    	if (node && node->exten)
     		snprintf(extenstr, sizeof(extenstr), "(%p)", node->exten);
     	if (strlen(node->x) > 1) {
     		ast_debug(1, "%s[%s]:%c:%c:%d:%s%s%s\n", prefix, node->x, node->is_pattern ? 'Y':'N',
     			node->deleted? 'D':'-', node->specificity, node->exten? "EXTEN:":"",
     	char extenstr[40];
     	struct ast_str *my_prefix = ast_str_alloca(1024);
     	extenstr[0] = '\0';
-    	if (node && node->exten && node->exten)
+    	if (node && node->exten)
     		snprintf(extenstr, sizeof(extenstr), "(%p)", node->exten);
     	if (strlen(node->x) > 1) {
     		ast_cli(fd, "%s[%s]:%c:%c:%d:%s%s%s\n", prefix, node->x, node->is_pattern ? 'Y' : 'N',
     			node->deleted ? 'D' : '-', node->specificity, node->exten? "EXTEN:" : "",
     		e3 = trie_find_next_match(m3);
     		if (e3) {
     			return e3;
+    		}
+    	}
     	return NULL;
+    }
     #ifdef DEBUG_THIS
     static char *action2str(enum ext_match_t action)
     		ast_log(LOG_NOTICE,"new_find_extension called with %s on (sub)tree %s action=%s\n", str, tree->x, action2str(action));
     	else
     		ast_log(LOG_NOTICE,"new_find_extension called with %s on (sub)tree NULL action=%s\n", str, action2str(action));
     #endif
     	for (p = tree; p; p = p->alt_char) {
     		if (p->is_pattern) {
-    		if (p->x[0] == 'N') {
+    			if (p->x[0] == 'N') {
-    			if (p->x[1] == 0 && *str >= '2' && *str <= '9' ) {
+    				if (p->x[1] == 0 && *str >= '2' && *str <= '9' ) {
-    #define NEW_MATCHER_CHK_MATCH	       \
+    #define	NEW_MATCHER_CHK_MATCH	       \
-    				if (p->exten && !(*(str + 1))) { /* if a shorter pattern matches along the way, might as well report it */           \
+    					if (p->exten && !(*(str + 1))) { /* if a shorter pattern matches along the way, might as well report it */             \
-    					if (action == E_MATCH || action == E_SPAWN || action == E_FINDLABEL) { /* if in CANMATCH/MATCHMORE, don't let matches get in the way */   \
+    						if (action == E_MATCH || action == E_SPAWN || action == E_FINDLABEL) { /* if in CANMATCH/MATCHMORE, don't let matches get in the way */   \
-    						update_scoreboard(score, length + 1, spec + p->specificity, p->exten, 0, callerid, p->deleted, p);           \
+    							update_scoreboard(score, length + 1, spec + p->specificity, p->exten, 0, callerid, p->deleted, p);                 \
-    						if (!p->deleted) {                                                                                           \
+    							if (!p->deleted) {                                                                                           \
-    							if (action == E_FINDLABEL) {                                                                             \
+    								if (action == E_FINDLABEL) {                                                                             \
-    								if (ast_hashtab_lookup(score->exten->peer_label_table, &pattern)) {                                  \
+    									if (ast_hashtab_lookup(score->exten->peer_label_table, &pattern)) {                                  \
-    									ast_debug(4, "Found label in preferred extension\n");                                            \
+    										ast_debug(4, "Found label in preferred extension\n");                                            \
-    									return;                                                                                          \
+    										return;                                                                                          \
-    								}                                                                                                    \
+    									}                                                                                                    \
-    							} else {                                                                                                 \
+    								} else {                                                                                                 \
-    								ast_debug(4,"returning an exact match-- first found-- %s\n", p->exten->exten);                       \
+    									ast_debug(4, "returning an exact match-- first found-- %s\n", p->exten->exten);                       \
-    								return; /* the first match, by definition, will be the best, because of the sorted tree */           \
+    									return; /* the first match, by definition, will be the best, because of the sorted tree */           \
-    							}                                                                                                        \
+    								}                                                                                                        \
-    						}                                                                                                            \
+    							}                                                                                                            \
-    					}                                                                                                                \
+    						}                                                                                                                \
+    					}
-    #define NEW_MATCHER_RECURSE	           \
+    #define	NEW_MATCHER_RECURSE	           \
-    				if (p->next_char && ( *(str + 1) || (p->next_char->x[0] == '/' && p->next_char->x[1] == 0)               \
+    					if (p->next_char && (*(str + 1) || (p->next_char->x[0] == '/' && p->next_char->x[1] == 0)                 \
-                                                     || p->next_char->x[0] == '!')) {                                        \
+            	                                       || p->next_char->x[0] == '!')) {                                          \
-    					if (*(str + 1) || p->next_char->x[0] == '!') {                                                       \
+    						if (*(str + 1) || p->next_char->x[0] == '!') {                                                         \
-    						new_find_extension(str + 1, score, p->next_char, length + 1, spec+p->specificity, callerid, label, action); \
+    							new_find_extension(str + 1, score, p->next_char, length + 1, spec + p->specificity, callerid, label, action); \
-    						if (score->exten)  {                                                                             \
+    							if (score->exten)  {                                                                             \
-    					        ast_debug(4, "returning an exact match-- %s\n", score->exten->exten);                        \
+    						        ast_debug(4 ,"returning an exact match-- %s\n", score->exten->exten);                         \
-    							return; /* the first match is all we need */                                                 \
+    								return; /* the first match is all we need */                                                 \
-    						}												                                                 \
+    							}												                                                 \
-    					} else {                                                                                             \
+    						} else {                                                                                             \
-    						new_find_extension("/", score, p->next_char, length + 1, spec+p->specificity, callerid, label, action);	 \
+    							new_find_extension("/", score, p->next_char, length + 1, spec + p->specificity, callerid, label, action);	 \
-    						if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {      \
+    							if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {      \
-    					        ast_debug(4,"returning a (can/more) match--- %s\n", score->exten ? score->exten->exten :     \
+    						        ast_debug(4,"returning a (can/more) match--- %s\n", score->exten ? score->exten->exten :     \
-                                           "NULL");                                                                          \
+            	                               "NULL");                                                                        \
-    							return; /* the first match is all we need */                                                 \
+    								return; /* the first match is all we need */                                                 \
-    						}												                                                 \
+    							}												                                                 \
-    					}                                                                                                    \
+    						}                                                                                                    \
-    				} else if (p->next_char && !*(str + 1)) {                                                                \
+    					} else if (p->next_char && !*(str + 1)) {                                                                  \
-    					score->canmatch = 1;                                                                                 \
+    						score->canmatch = 1;                                                                                 \
-    					score->canmatch_exten = get_canmatch_exten(p);                                                       \
+    						score->canmatch_exten = get_canmatch_exten(p);                                                       \
-    					if (action == E_CANMATCH || action == E_MATCHMORE) {                                                 \
+    						if (action == E_CANMATCH || action == E_MATCHMORE) {                                                 \
-    				        ast_debug(4, "returning a canmatch/matchmore--- str=%s\n", str);                                 \
+    					        ast_debug(4, "returning a canmatch/matchmore--- str=%s\n", str);                                  \
-    						return;                                                                                          \
+    							return;                                                                                          \
-    					}												                                                     \
+    						}												                                                     \
+    					}
-    				NEW_MATCHER_CHK_MATCH;
+    					NEW_MATCHER_CHK_MATCH;
-    				NEW_MATCHER_RECURSE;
+    					NEW_MATCHER_RECURSE;
+    				}
-    		} else if (p->x[0] == 'Z') {
+    			} else if (p->x[0] == 'Z') {
-    			if (p->x[1] == 0 && *str >= '1' && *str <= '9' ) {
+    				if (p->x[1] == 0 && *str >= '1' && *str <= '9' ) {
-    				NEW_MATCHER_CHK_MATCH;
+    					NEW_MATCHER_CHK_MATCH;
-    				NEW_MATCHER_RECURSE;
+    					NEW_MATCHER_RECURSE;
+    				}
-    		} else if (p->x[0] == 'X') {
+    			} else if (p->x[0] == 'X') {
-    			if (p->x[1] == 0 && *str >= '0' && *str <= '9' ) {
+    				if (p->x[1] == 0 && *str >= '0' && *str <= '9' ) {
-    				NEW_MATCHER_CHK_MATCH;
+    					NEW_MATCHER_CHK_MATCH;
-    				NEW_MATCHER_RECURSE;
+    					NEW_MATCHER_RECURSE;
+    				}
-    		} else if (p->x[0] == '.' && p->x[1] == 0) {
+    			} else if (p->x[0] == '.' && p->x[1] == 0) {
-    			/* how many chars will the . match against? */
+    				/* how many chars will the . match against? */
-    			int i = 0;
+    				int i = 0;
-    			const char *str2 = str;
+    				const char *str2 = str;
-    			while (*str2 && *str2 != '/') {
+    				while (*str2 && *str2 != '/') {
-    				str2++;
+    					str2++;
-    				i++;
+    					i++;
+    				}
-    			if (p->exten && *str2 != '/') {
+    				if (p->exten && *str2 != '/') {
-    				update_scoreboard(score, length+i, spec+(i*p->specificity), p->exten, '.', callerid, p->deleted, p);
+    					update_scoreboard(score, length + i, spec + (i * p->specificity), p->exten, '.', callerid, p->deleted, p);
-    				if (score->exten) {
+    					if (score->exten) {
-    					ast_debug(4,"return because scoreboard has a match with '/'--- %s\n", score->exten->exten);
+    						ast_debug(4,"return because scoreboard has a match with '/'--- %s\n", score->exten->exten);
-    					return; /* the first match is all we need */
+    						return; /* the first match is all we need */
+    					}
+    				}
-    			if (p->next_char && p->next_char->x[0] == '/' && p->next_char->x[1] == 0) {
+    				if (p->next_char && p->next_char->x[0] == '/' && p->next_char->x[1] == 0) {
-    				new_find_extension("/", score, p->next_char, length+i, spec+(p->specificity*i), callerid, label, action);
+    					new_find_extension("/", score, p->next_char, length + i, spec+(p->specificity*i), callerid, label, action);
-    				if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
+    					if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
-    					ast_debug(4, "return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set--- %s\n", score->exten ? score->exten->exten : "NULL");
+    						ast_debug(4, "return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set--- %s\n", score->exten ? score->exten->exten : "NULL");
-    					return; /* the first match is all we need */
+    						return; /* the first match is all we need */
+    					}
+    				}
-    			while (*str2 && *str2 != '/') {
+    				while (*str2 && *str2 != '/') {
-    				str2++;
+    					str2++;
-    				i++;
+    					i++;
+    				}
-    			if (p->exten && *str2 != '/') {
+    				if (p->exten && *str2 != '/') {
-    				update_scoreboard(score, length + 1, spec+(p->specificity * i), p->exten, '!', callerid, p->deleted, p);
+    					update_scoreboard(score, length + 1, spec + (p->specificity * i), p->exten, '!', callerid, p->deleted, p);
-    				if (score->exten) {
+    					if (score->exten) {
-    					ast_debug(4, "return because scoreboard has a '!' match--- %s\n", score->exten->exten);
+    						ast_debug(4, "return because scoreboard has a '!' match--- %s\n", score->exten->exten);
-    					return; /* the first match is all we need */
+    						return; /* the first match is all we need */
+    					}
+    				}
-    			if (p->next_char && p->next_char->x[0] == '/' && p->next_char->x[1] == 0) {
+    				if (p->next_char && p->next_char->x[0] == '/' && p->next_char->x[1] == 0) {
-    				new_find_extension("/", score, p->next_char, length + i, spec + (p->specificity * i), callerid, label, action);
+    					new_find_extension("/", score, p->next_char, length + i, spec + (p->specificity * i), callerid, label, action);
-    				if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
+    					if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
-    					ast_debug(4,"return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set with '/' and '!'--- %s\n", score->exten ? score->exten->exten : "NULL");
+    						ast_debug(4, "return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set with '/' and '!'--- %s\n", score->exten ? score->exten->exten : "NULL");
-    					return; /* the first match is all we need */
+    						return; /* the first match is all we need */
+    					}
+    				}
-    		} else if (p->x[0] == '/' && p->x[1] == 0) {
+    			} else if (p->x[0] == '/' && p->x[1] == 0) {
-    			/* the pattern in the tree includes the cid match! */
+    				/* the pattern in the tree includes the cid match! */
-    			if (p->next_char && callerid && *callerid) {
+    				if (p->next_char && callerid && *callerid) {
-    				new_find_extension(callerid, score, p->next_char, length+1, spec, callerid, label, action);
+    					new_find_extension(callerid, score, p->next_char, length + 1, spec, callerid, label, action);
-    				if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
+    					if (score->exten || ((action == E_CANMATCH || action == E_MATCHMORE) && score->canmatch)) {
-    					ast_debug(4, "return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set with '/'--- %s\n", score->exten ? score->exten->exten : "NULL");
+    						ast_debug(4, "return because scoreboard has exact match OR CANMATCH/MATCHMORE & canmatch set with '/'--- %s\n", score->exten ? score->exten->exten : "NULL");
-    					return; /* the first match is all we need */
+    						return; /* the first match is all we need */
+    					}
+    				}
-    		} else if (strchr(p->x, *str)) {
+    			} else if (strchr(p->x, *str)) {
-    			ast_debug(4, "Nothing strange about this match\n");
+    				ast_debug(4, "Nothing strange about this match\n");
-    			NEW_MATCHER_CHK_MATCH;
+    				NEW_MATCHER_CHK_MATCH;
-    			NEW_MATCHER_RECURSE;
+    				NEW_MATCHER_RECURSE;
+    			}
     		} else if (strchr(p->x, *str)) {
     			ast_debug(4, "Nothing strange about this match\n");
     			NEW_MATCHER_CHK_MATCH;
     			NEW_MATCHER_RECURSE;
+    		}
+    	}
     	ast_debug(4, "return at end of func\n");
+    }
     /* the algorithm for forming the extension pattern tree is also a bit simple; you
      * that a regex only handles 1 pattern, really. This trie holds any number
      * of patterns. Well, really, it **could** be considered a single pattern,
      * where the "|" (or) operator is allowed, I guess, in a way, sort of...
      */
-    static struct match_char *already_in_tree(struct match_char *current, char *pat)
+    static struct match_char *already_in_tree(struct match_char *current, char *pat, int is_pattern)
+    {
     	struct match_char *t;
     	if (!current) {
     		return 0;
+    	}
     	for (t = current; t; t = t->alt_char) {
-    		if (!strcmp(pat, t->x)) { /* uh, we may want to sort exploded [] contents to make matching easy */
+    		if (is_pattern == t->is_pattern && !strcmp(pat, t->x)) {/* uh, we may want to sort exploded [] contents to make matching easy */
     			return t;
+    		}
+    	}
     	return 0;
     	/* insert node into the tree at "current", so the alt_char list from current is
     	   sorted in increasing value as you go to the leaves */
     	if (!(*parent_ptr)) {
     		*parent_ptr = node;
-    	} else {
+    		return;
     		if ((*parent_ptr)->specificity > node->specificity) {
     	if ((*parent_ptr)->specificity > node->specificity){
-    			/* insert at head */
+    		/* insert at head */
-    			node->alt_char = (*parent_ptr);
+    		node->alt_char = (*parent_ptr);
-    			*parent_ptr = node;
+    		*parent_ptr = node;
-    		} else {
+    		return;
+    	}
-    			lcurr = *parent_ptr;
+    	lcurr = *parent_ptr;
-    			for (curr = (*parent_ptr)->alt_char; curr; curr = curr->alt_char) {
+    	for (curr = (*parent_ptr)->alt_char; curr; curr = curr->alt_char) {
-    				if (curr->specificity > node->specificity) {
+    		if (curr->specificity > node->specificity) {
-    					node->alt_char = curr;
+    			node->alt_char = curr;
-    					lcurr->alt_char = node;
+    			lcurr->alt_char = node;
-    					break;
+    			break;
+    		}
-    				lcurr = curr;
+    		lcurr = curr;
+    	}
-    			if (!curr) {
+    	if (!curr) {
-    				lcurr->alt_char = node;
+    		lcurr->alt_char = node;
+    	}
+    }
     static struct match_char *add_pattern_node(struct ast_context *con, struct match_char *current, char *pattern, int is_pattern, int already, int specificity, struct match_char **nextcharptr)
+    {
     	struct match_char *m;
-    	if (!(m = ast_calloc(1, sizeof(*m)))) {
+    	if (!(m = ast_calloc(1, sizeof(*m) + strlen(pattern)))) {
     		return NULL;
+    	}
-    	if (!(m->x = ast_strdup(pattern))) {
+    	/* strcpy is safe here since we know its size and have allocated
-    		ast_free(m);
+    	 * just enough space for when we allocated m
-    		return NULL;
+    	 */
     	strcpy(m->x, pattern);
     	/* the specificity scores are the same as used in the old
     	   pattern matcher. */
     	m->is_pattern = is_pattern;
     	if (specificity == 1 && is_pattern && pattern[0] == 'N')
     	if ( *s1 == '_') {
     		pattern = 1;
     		s1++;
+    	}
-    	while( *s1 ) {
+    	while (*s1) {
-    		if (pattern && *s1 == '[' && *(s1-1) != '\\') {
+    		if (pattern && *s1 == '[' && *(s1 - 1) != '\\') {
     			char *s2 = buf;
     			buf[0] = 0;
     			s1++; /* get past the '[' */
-    			while (*s1 != ']' && *(s1 - 1) != '\\' ) {
+    			while (*s1 != ']' && *(s1 - 1) != '\\') {
     				if (*s1 == '\\') {
     					if (*(s1 + 1) == ']') {
     						*s2++ = ']';
-    						s1++; s1++;
+    						s1 += 2;
     					} else if (*(s1 + 1) == '\\') {
     						*s2++ = '\\';
-    						s1++; s1++;
+    						s1 += 2;
     					} else if (*(s1 + 1) == '-') {
     						*s2++ = '-';
-    						s1++; s1++;
+    						s1 += 2;
     					} else if (*(s1 + 1) == '[') {
     						*s2++ = '[';
-    						s1++; s1++;
+    						s1 += 2;
+    					}
     				} else if (*s1 == '-') { /* remember to add some error checking to all this! */
     					char s3 = *(s1 - 1);
     					char s4 = *(s1 + 1);
     					for (s3++; s3 <= s4; s3++) {
     						*s2++ = s3;
+    					}
-    					s1++; s1++;
+    					s1 += 2;
     				} else if (*s1 == '\0') {
     					ast_log(LOG_WARNING, "A matching ']' was not found for '[' in pattern string '%s'\n", extenbuf);
     					break;
     				} else {
     					*s2++ = *s1++;
     			specif = strlen(buf);
     			qsort(buf, specif, 1, compare_char);
     			specif <<= 8;
     			specif += buf[0];
     		} else {
     			if (*s1 == '\\') {
     				s1++;
     				buf[0] = *s1;
     			} else {
     				if (pattern) {
-    					if (*s1 == 'n') /* make sure n,x,z patterns are canonicalized to N,X,Z */
+    					if (*s1 == 'n') { /* make sure n,x,z patterns are canonicalized to N,X,Z */
     						*s1 = 'N';
-    					else if (*s1 == 'x')
+    					} else if (*s1 == 'x') {
     						*s1 = 'X';
-    					else if (*s1 == 'z')
+    					} else if (*s1 == 'z') {
     						*s1 = 'Z';
+    					}
+    				}
     				buf[0] = *s1;
+    			}
     			buf[1] = 0;
     			specif = 1;
+    		}
     		m2 = 0;
-    		if (already && (m2 = already_in_tree(m1,buf)) && m2->next_char) {
+    		if (already && (m2 = already_in_tree(m1, buf, pattern)) && m2->next_char) {
     			if (!(*(s1 + 1))) {  /* if this is the end of the pattern, but not the end of the tree, then mark this node with the exten...
-    								  * a shorter pattern might win if the longer one doesn't match */
+    								a shorter pattern might win if the longer one doesn't match */
     				if (m2->exten) {
     					ast_log(LOG_WARNING, "Found duplicate exten. Had %s found %s\n", m2->exten->exten, e1->exten);
+    				}
     				m2->exten = e1;
     				m2->deleted = 0;
+    			}
     			m1 = m2->next_char; /* m1 points to the node to compare against */
     			m0 = &m2->next_char; /* m0 points to the ptr that points to m1 */
     				m1 = m2; /* while m0 stays the same */
     			} else {
     				if (findonly) {
     					return m1;
+    				}
-    				m1 = add_pattern_node(con, m1, buf, pattern, already,specif, m0); /* m1 is the node just added */
+    				if (!(m1 = add_pattern_node(con, m1, buf, pattern, already,specif, m0))) { /* m1 is the node just added */
     					return NULL;
+    				}
     				m0 = &m1->next_char;
+    			}
     			if (!(*(s1 + 1))) {
     				if (m2) {
     					ast_log(LOG_WARNING, "Found duplicate exten. Had %s found %s\n", m2->exten->exten, e1->exten);
+    				}
     				m1->deleted = 0;
     				m1->exten = e1;
+    			}
     			/* The 'already' variable is a mini-optimization designed to make it so that we
     			 * don't have to call already_in_tree when we know it will return false.
     			 */
     			already = 0;
+    		}
     		s1++; /* advance to next char */
+    	}
     	return m1;
     	struct ast_hashtab_iter *t1;
     	struct ast_exten *e1;
     #ifdef NEED_DEBUG
     	int biggest_bucket, resizes, numobjs, numbucks;
-    	ast_log(LOG_DEBUG,"Creating Extension Trie for context %s\n", con->name);
+    	ast_log(LOG_DEBUG,"Creating Extension Trie for context %s(%p)\n", con->name, con);
     	ast_hashtab_get_stats(con->root_table, &biggest_bucket, &resizes, &numobjs, &numbucks);
     	ast_log(LOG_DEBUG,"This tree has %d objects in %d bucket lists, longest list=%d objects, and has resized %d times\n",
     			numobjs, numbucks, biggest_bucket, resizes);
     #endif
     	t1 = ast_hashtab_start_traversal(con->root_table);
     	if (pattern_tree->next_char) {
     		destroy_pattern_tree(pattern_tree->next_char);
     		pattern_tree->next_char = 0;
+    	}
     	pattern_tree->exten = 0; /* never hurts to make sure there's no pointers laying around */
-    	if (pattern_tree->x) {
+    	ast_free(pattern_tree);
-    		free(pattern_tree->x);
-    	free(pattern_tree);
+    }
     /*
      * Special characters used in patterns:
      *	'_'	underscore is the leading character of a pattern.
     		struct fake_context item;
     		ast_copy_string(item.name, context, sizeof(item.name));
     		tmp = ast_hashtab_lookup(contexts_table, &item);
-    #ifdef NOTNOW
-    		tmp = NULL;
-    		while ((tmp = ast_walk_contexts(tmp)) ) {
-    			if (!strcmp(tmp->name, context)) {
-    				break;
-    #endif
     		if (!tmp) {
     			return NULL;
+    		}
+    	}
     					q->status = STATUS_NO_LABEL;
     				e = ast_hashtab_lookup(eroot->peer_label_table, &pattern);
     			} else {
     				e = ast_hashtab_lookup(eroot->peer_table, &pattern);
+    			}
-    #ifdef NOTNOW
-    			while ( (e = ast_walk_extension_priorities(eroot, e)) ) {
-    				/* Match label or priority */
-    				if (action == E_FINDLABEL) {
-    					if (q->status < STATUS_NO_LABEL)
-    						q->status = STATUS_NO_LABEL;
-    					if (label && e->label && !strcmp(label, e->label))
-    						break;	/* found it */
-    				} else if (e->priority == priority) {
-    					break;	/* found it */
-    				} /* else keep searching */
-    #endif
     			if (e) {	/* found a valid match */
     				q->status = STATUS_SUCCESS;
     				q->foundcontext = context;
     				return e;
+    			}
     	ast_copy_string(item.name, context, sizeof(item.name));
     	ast_rdlock_contexts();
     	c = ast_hashtab_lookup(contexts_table,&item);
-    #ifdef NOTNOW
-    	while ( (c = ast_walk_contexts(c)) ) {
-    		if (!strcmp(ast_get_context_name(c), context))
-    			return c;
-    #endif
     	if (!c)
     		ast_unlock_contexts();
     	return c;
+    }
     	ast_copy_string(item.name, context, sizeof(item.name));
     	c = ast_hashtab_lookup(contexts_table,&item);
     	if (c)
     		ret = 0;
-    #ifdef NOTNOW
-    	while ((c = ast_walk_contexts(c))) {
-    		if (!strcmp(ast_get_context_name(c), context)) {
-    			ret = 0;
-    			break;
-    #endif
     	ast_unlock_contexts();
     	/* if we found context, lock macrolock */
     	if (ret == 0) {
     		ret = ast_mutex_lock(&c->macrolock);
     	ast_copy_string(item.name, context, sizeof(item.name));
     	c = ast_hashtab_lookup(contexts_table,&item);
     	if (c)
     		ret = 0;
-    #ifdef NOTNOW
-    	while ((c = ast_walk_contexts(c))) {
-    		if (!strcmp(ast_get_context_name(c), context)) {
-    			ret = 0;
-    			break;
-    #endif
     	ast_unlock_contexts();
     	/* if we found context, unlock macrolock */
     	if (ret == 0) {
     		ret = ast_mutex_unlock(&c->macrolock);

/trunk/main/pbx.c:

You have a pending review.

Other reviews

Your comment

Review Board 1.7.16

This change has been marked as submitted.

Various fixes for extenpatternmatchnew

Diff revision 1 (Latest)