Several fixes to the extenpatternmatchnew logic.

1. Differentiate between literal characters in an extension
and characters that should be treated as a pattern match. Prior to
these fixes, an extension such as NNN would be treated as a pattern,
rather than a literal string of N's.

2. Fixed the logic used when matching an extension with a bracketed
expression, such as 2[5-7]6.

3. Removed all areas of code that were executed when NOT_NOW was
#defined. The code in these areas had the potential to crash, for
one thing, and the actual intent of these blocks seemed counterproductive.

4. Fixed many many coding guidelines problems I encountered while looking
through the corresponding code.

5. Added failure cases and warning messages for when duplicate extensions
are encountered.

6. Miscellaneous fixes to incorrect or redundant statements.

(closes issue #14615)
Reported by: steinwej
Tested by: mmichelson

Review: http://reviewboard.digium.com/r/194/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@188901 65c4cc65-6c06-0410-ace0-fbb531ad65f3

main/pbx.c

@@ -818,11 +818,11 @@ 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 */
@@ -949,7 +949,7 @@ 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,
@@ -1497,7 +1497,7 @@ void log_match_char_tree(struct match_char *node, char *prefix)
 
 	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) {
@@ -1526,7 +1526,7 @@ static void cli_match_char_tree(struct match_char *node, char *prefix, int fd)
 
 	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) {
@@ -1601,6 +1601,7 @@ static struct ast_exten *trie_find_next_match(struct match_char *node)
 			return e3;
 		}
 	}
+
 	return NULL;
 }
 
@@ -1636,6 +1637,7 @@ static void new_find_extension(const char *str, struct scoreboard *score, struct
 		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[1] == 0 && *str >= '2' && *str <= '9' ) {
 #define	NEW_MATCHER_CHK_MATCH	       \
@@ -1753,6 +1755,11 @@ static void new_find_extension(const char *str, struct scoreboard *score, struct
 				NEW_MATCHER_CHK_MATCH;
 				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");
 }
@@ -1774,7 +1781,7 @@ static void new_find_extension(const char *str, struct scoreboard *score, struct
  * 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;
 
@@ -1783,7 +1790,7 @@ static struct match_char *already_in_tree(struct match_char *current, char *pat)
 	}
 
 	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;
 		}
 	}
@@ -1803,12 +1810,16 @@ static void insert_in_next_chars_alt_char_list(struct match_char **parent_ptr, s
 	   sorted in increasing value as you go to the leaves */
 	if (!(*parent_ptr)) {
 		*parent_ptr = node;
-	} else {
+		return;
+	}
+
 	if ((*parent_ptr)->specificity > node->specificity){
 		/* insert at head */
 		node->alt_char = (*parent_ptr);
 		*parent_ptr = node;
-		} else {
+		return;
+	} 
+
 	lcurr = *parent_ptr;
 	for (curr = (*parent_ptr)->alt_char; curr; curr = curr->alt_char) {
 		if (curr->specificity > node->specificity) {
@@ -1818,27 +1829,25 @@ static void insert_in_next_chars_alt_char_list(struct match_char **parent_ptr, s
 		}
 		lcurr = curr;
 	}
+
 	if (!curr) {
 		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. */
@@ -1910,16 +1919,16 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str
 				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);
@@ -1927,7 +1936,7 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str
 					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;
@@ -1943,28 +1952,31 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str
 			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;
 			}
@@ -1980,15 +1992,22 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str
 				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 */
@@ -2003,7 +2022,7 @@ static void create_match_char_tree(struct ast_context *con)
 #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);
@@ -2032,10 +2051,7 @@ static void destroy_pattern_tree(struct match_char *pattern_tree) /* pattern tre
 		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);
 }
 
 /*
@@ -2494,14 +2510,6 @@ struct ast_exten *pbx_find_extension(struct ast_channel *chan,
 		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;
 		}
@@ -2684,19 +2692,6 @@ struct ast_exten *pbx_find_extension(struct ast_channel *chan,
 			} 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;
@@ -4633,13 +4628,6 @@ static struct ast_context *find_context_locked(const char *context)
 	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();
 
@@ -4966,18 +4954,6 @@ int ast_context_lockmacro(const char *context)
 	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 */
@@ -5006,16 +4982,6 @@ int ast_context_unlockmacro(const char *context)
 	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 */