[asterisk-dev] [Code Review] 2930: pbx.c: eliminate confusion when matching caller id and prevent removal of extension entry still in hashtable

[Guenther Kelleter]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2930/#review9967
-----------------------------------------------------------


Scott,
all I can offer you is to test this patch if it fixes the original issue as I don't understand how this context extension tree works. So if you introduced a new bug in the code I won't see it.

- Guenther Kelleter


On Oct. 21, 2013, 11:18 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2930/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2013, 11:18 p.m.)
> 
> 
> Review request for Asterisk Developers and Guenther Kelleter.
> 
> 
> Bugs: AST-1235
>     https://issues.asterisk.org/jira/browse/AST-1235
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch fixes a bug report where restarting Asterisk repeatedly would cause a crash due to accidentally deleting extensions that were still referenced in the context's root_table.  This occurred due to confusion over matching or not matching extensions based on callerid, which is also being cleared up via an enum'd third state.
> 
> There are two values in an extension entry:  matchcid which says whether cid should be matched, and cidmatch which is the string value to match.  The matchcid value was previously non-zero when cidmatch was signficant.  This is important since a null length callerid match must be kept distinct from a match-anything condition.
> 
> This patch adds an enum to define the AST_EXT_MATCHCID_OFF (0) or AST_EXT_MATCHCID_ON (1) behaviors, and adds AST_EXT_MATCHCID_ANY (2) which allows matching to both exten->matchcid=0 and exten->matchcid=1 cases.  For example, all exten's with the same exten (number) can be deleted with ANY, but when looking for a dialplan match it must match to matchcid, and if set to ON, further match cidmatch.
> 
> The logic to match extensions has been reworked to implement these three types, and then matchcid values updated to use the enum for clarity.
> 
> The most significant change is in ast_context_remove_extension_callerid2() where after removing extensions from the hashtable, the deletion would destroy the extensions.  This was deleting exten's that didn't have matchcid set but matched a null cidmatch, where they hadn't been removed from the hashtable.
> 
> - (!callerid || !matchcallerid || (matchcallerid && !strcmp(peer->cidmatch, callerid))) &&
> + (!callerid || (!matchcallerid && !peer->matchcid) || (matchcallerid && peer->matchcid && !strcmp(peer->cidmatch, callerid))) &&
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/include/asterisk/pbx.h 400531 
>   /branches/1.8/main/pbx.c 400531 
> 
> Diff: https://reviewboard.asterisk.org/r/2930/diff/
> 
> 
> Testing
> -------
> 
> This patch has been tested with the testsuite additions dialplan_reload and pbx/callerid_match (posted separately) to insure that the crash case has been resolved without any other unwanted changes in behavior.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131022/c0840bc3/attachment.html>