Review Board 1.7.16


Add ability to set Max-Forwards header from dialplan, general and device configuration

Review Request #778 - Created July 13, 2010 and submitted

Olle E Johansson
/team/oej/sip-max-forwards-trunk
Reviewers
asterisk-dev
Asterisk
This change adds the ability to set the value of the Max-Forwards header from the Asterisk dialplan and from sip.conf - the general section and per device. 
The 1.4 version has been in use with the dialplan support for a few weeks. The device config is new.
Ship it!
Posted (July 13, 2010, 8:11 a.m.)
Nice work!  I just had a few minor comments.
/trunk/channels/chan_sip.c (Diff revision 1)
 
 
Using sscanf will let you detect a config error here which seems like it would be a good idea.  If this isn't a valid unsigned int, a log error should be generated and the default should remain.
  1. Good suggestions, thanks.
/trunk/configs/sip.conf.sample (Diff revision 1)
 
 
It might be useful explicitly state what the default value is here when this option is not enabled.
  1. Fixed.
Posted (Aug. 11, 2010, 7:15 a.m.)
I reopened this review as I found a deadlock caused by this code.
/trunk/channels/chan_sip.c (Diff revision 1)
 
 
 
 
Deadlock avoidance is needed here.  It is possible for add_header_max_forwards() to be called with the pvt locked but not the channel; pbx_builtin_getvar_helper() then attempts to lock the channel lock, which may already be held by something else waiting for the pvt lock (ast_indicate, ast_hangup, etc).  This causes a deadlock. I have seen this in the blind-transfer-accountcode test on my test box.
  1. Disregard this, I'll commit the fix myself.

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.