Summary:	ASTERISK-06897: [patch] fix for unnecessary dial delay if 2 or more early match patterns (i.e. with !) are present
Reporter:	Michael Neuhauser (mneuhauser)	Labels:
Date Opened:	2006-05-04 15:36:26	Date Closed:	2006-05-16 05:41:07
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Core/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:
Description:	Case: a context has 2 (or more) early match patterns like these:    _9X! =>    _X! => When "90" is dialled, the first pattern matches only after a short delay (matchdigittimeout in chan_zap) instead of immediatly after the "0" was dialled. Immediate matching does happen if the 2nd pattern is not present. I believe to have this tracked down to a small buglet in pbx_find_extension():    if (action == HELPER_MATCHMORE && match == 2 && !earlymatch) { /* It matched an extension ending in a '!' wildcard   So ignore it for now, unless there's a better match */ earlymatch = eroot;  } else { The match on the first early pattern is ignored, but not the 2nd one (as "earlymatch" was already set). The problem is, that the 2nd pattern is handled in the else branch, and a value != NULL is returned. The solution (which works in the above mentioned case), is to ignore ALL early matches, not only the first one:  if (action == HELPER_MATCHMORE && match == 2) { /* It matched an extension ending in a '!' wildcard   So ignore it for now, unless there's a better match */ earlymatch = eroot;  } else { I believe this change to be safe, as nothing ever is done with the value of "earlymatch", except returning NULL later on if earlymatch != NULL (maybe using a flag instead of a pointer, i.e. "int had_earlymatch; ... had_earlymatch = 1;" would be clearer).
Comments:	By: Serge Vecher (serge-v) 2006-05-04 15:42:08 1) please get a disclaimer on file 2) upload the path in diff -u format Thanks. By: Michael Neuhauser (mneuhauser) 2006-05-04 15:46:07 Short bugfix -> no disclaimer necessary. regarding patch: You gotta be kidding me. I've spend hours tracking this down, and you are not willing to delete 15 characters by hand? By: Luigi Rizzo (rizzo) 2006-05-10 15:11:02 Although i independently put a similar comment in the code in my branch (now in trunk), i am not 100% sure the patch you propose implements the intended behaviour, and also, in this specific case, there is another issue related to the sorting of patterns in the contents. Since we recently changed the latter, i would be grateful if you (mneuhauser) could let me know if the problem still occurs with the version of pbx.c now in trunk - the sorting order has been changed and now more specific ones come first, so you should match the _9X! immediately. By: Michael Neuhauser (mneuhauser) 2006-05-11 04:13:25 Due to various reasons I can not run trunk, sorry. But for the example presentedby me (_X!, _9X!, exten=90), the sort order does not matter at all as both matches are early matches (and pbx_find_extension() keeps on matching for early matches in MATCHMORE mode). As I understand it, NULL should be returned for MATCHMORE if the exten is already long enough and no more digits are needed. Think of it this way: if it is OK to return NULL for exten=90 and only the pattern "_9X!" is present, why should it not be OK to do so if "_X!" is also present? The second ! pattern encountered is handled in the else branch (because earlymatch is already set) and there a value != NULL is returned. I'm pretty sure that the else branch must never be entered for MATCHMORE and match == 2. But I'm not so sure that it is OK to return NULL immediatly (your comment in thetrunk) when a MATCHMORE/early match is encountered. I think that depends on the sort order of the patterns and the intented behaviour of "!": 1) match as early as possible, even if there are longer/more specific patterns   that could match if there were more digits 2) match as soon as it sure that no longer/more specific pattern could match if   there were more digits (i.e. same as "." but without digittimeout) I always assumed that 2) is the case, e.g. with only the patterns "_1!" and "_12!" and exten="1", wait for another digit or timeout so that the approbiate pattern can be mached, but for exten="13" match "_1!" immediatly and do not wait for another digit/timeout (as it would be the case for "_1."). So if 2) is the case => it is OK to immediatly return NULL for MATCHMORE/early-match if the sort order of the patterns guarantees that no longer/more specific pattern could be encountered later on. I think the main point is: indepentend of pattern sort order, it must be guaranteed that the else branch is not entered for early matches in MATCHMORE mode. By: Luigi Rizzo (rizzo) 2006-05-15 01:55:05 mneuhauser: your bottom sententence is absolutely correct and the code has been fixed. Unless you have objections, i think this bug can be closed. For the rest, we specified (and documented in pbx.c) a sorting order of extensions in the dialplan so that it was clear to the users what to expect from their dialplans. I cannot comment on whether the way ! is specified (in relation to its use with MATCHMORE or CANMATCH) is useful or not because i am not familiar with how CANMATCH and MATCHMORE are used in the code. By: Michael Neuhauser (mneuhauser) 2006-05-16 05:33:32 rizzo: closing the bug is OK with me By: Luigi Rizzo (rizzo) 2006-05-16 05:41:07 issue resolved. it would be good to put in the 1.2 branch the change (remove !earlymatch from the test) proposed in the original description, which is so trivial that doesn't require a disclaimer.