[asterisk-dev] [Code Review] Manage translation table between SIP and ISDN hangup causes

[Matt Jordan]

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


So I have no idea why the svn merge didn't pull over the new files.  I ended up just checking out earl-grey and looking over sip2cause.c.

Line 41, sip2causestruct definition - right now you have this structured as a list of sip2causestruct objects.  This means to find a specific entry, you have to iterate over the entire list of sip2causestruct definitions in a table.

A possible faster way of structuring this data would be to use an ao2 object with the same definition of sip2causestruct, minus the 'next' pointer.  You could then modify sip2causetable to contain a container of those objects:

struct sip2causetable {
   struct ao2_container *table;
   int default_code;
   AST_DECLARE_STRING_FIELDS(
      AST_STRING_FIELD(default_reason);
   );
};

You would then create two different instances of sip2causetable - one would be for your SIP to ISDN mappings, and one would be for your ISDN to SIP mappings.  They would differ in their key lookups - one would key off of sip2causestruct's sip field, the other would key off of sipcausestruct's cause field.  This lets you do O(1) lookups of the appropriate mappings instead of O(n) lookups, which should dramatically speed up hangup_sip2cause and hangup_cause2sip.

As an aside, I don't see where defaultcode/defaultreason are used in sip2causetable.  If they aren't needed, this simplifies things even more - you can get rid of the sip2causetable struct completely and replace it with two ao2_containers:

static struct ao2_container *sip2cause_lookups;
static struct ao2_container *cause2sip_lookups;

(As an aside, more use of the '_' please :-))

- Matt


On Dec. 5, 2012, 5:34 a.m., Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2227/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2012, 5:34 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The SIP2CAUSE hangup code conversion tables has up to now been hard-coded in Asterisk. In some cases, like when building in-house ISDN/Q.SIG to SIP gateways, there's a need to manipulate this conversion. 
> 
> With this code, advanced users can add a "private" conversion. This is added in front of the built-in conversions.
> 
> Asterisk conversion tables does not change in this patch. Everything should work as before. To shrink the chan_sip.c file a small bit I decided to move this functionality into a new source code file.
> 
> Adding:
> - new source code file sip2cause.c and include file sip2cause.h
> - new configuration file sip2cause.conf
> 
> Reviewboard doesn't seem accept the new files, so they have to be found in the branch itself.
> 
> http://svn.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk
> 
> The new files are:
> * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/configs/sip2cause.conf.sample
> * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/sip2cause.c
> * http://svnview.digium.com/svn/asterisk/team/oej/earl-grey-sip2cause-configurable-trunk/channels/sip/include/sip2cause.h
> 
> 
> This addresses bug ASTERISK-20759.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20759
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 377205 
>   /trunk/channels/sip/include/sip_utils.h 377205 
> 
> Diff: https://reviewboard.asterisk.org/r/2227/diff
> 
> 
> Testing
> -------
> 
> Tested all kinds of weird translations. This file should cause some errors (AST_CAUSE_SKREP doesn't exist, 903 is not a valid SIP reason code etc etc. 
> 
> [sip2cause]
> 604 => AST_CAUSE_SKREP
> 404 => UNALLOCATED
> 599 Bad => USER_BUSY
> 486 => NORMAL_CLEARING
> 603 => UNALLOCATED
>         
> [cause2sip]
> SKREP => 503 Service Failure
> UNALLOCATED => 903 Go to hell
> UNALLOCATED => 499 I don't want to do that.
> USER_BUSY => 503 I am not feeling well
> 
> 
> Thanks,
> 
> Olle E
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121206/42fecb3c/attachment.htm>