[asterisk-dev] [Code Review] 4467: res_pjsip: Fix pjsip.conf type=global object default value handling.

[rmudgett]

> On March 10, 2015, 8:25 a.m., Joshua Colp wrote:
> > /branches/13/res/res_pjsip/config_global.c, line 229
> > <https://reviewboard.asterisk.org/r/4467/diff/1/?file=71922#file71922line229>
> >
> >     Why the change of this from a define to not a device?
> 
> rmudgett wrote:
>     The define did not help readability, required you to look elsewhere to figure out the generated string, and due to the generated string's nature could only be used once.
> 
> Matt Jordan wrote:
>     I disagree with this change. Generally having a constant located in a #define is a good thing. This is also one of those pieces of data that some people will want to customize - having them have to go dig into the config_global file when it was easily located at the top with the other #define'd constants feels like a step backwards.

Restoring.  That makes sense.


- rmudgett


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


On March 10, 2015, 10:48 a.m., rmudgett wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4467/
> -----------------------------------------------------------
> 
> (Updated March 10, 2015, 10:48 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24807
>     https://issues.asterisk.org/jira/browse/ASTERISK-24807
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When a type=global section is not defined in pjsip.conf the global
> defaults are not applied.  As a result the mandatory Max-Forwards header
> is not added to SIP messages for res_pjsip/chan_pjsip.
> 
> The handling of pjsip.conf type=global objects has several problems:
> 
> 1) If the global object is missing the defaults are not applied.
> 
> 2) If the global object is missing the default_outbound_endpoint's default
> value is not returned by ast_sip_global_default_outbound_endpoint().
> 
> 3) Defines are needed so default values only need to be changed in one
> place.
> 
> * Added a sorcery instance observer callback to check if there were any
> type=global sections loaded.  If there were more than one then issue an
> error message.  If there were none then apply the global defaults.
> 
> * Fixed ast_sip_global_default_outbound_endpoint() to return the
> documented default when no type=global object is defined.
> 
> * Made defines for the global default values.
> 
> * Increased the default_useragent[] size because SVN version strings can
> get lengthy and 128 characters may not be enough.
> 
> * Fixed an off-nominal code path ref leak in global_alloc() if the string
> fields fail to initialize.
> 
> * Eliminated RAII_VAR in get_global_cfg() and
> ast_sip_global_default_outbound_endpoint().
> 
> 
> The changes to res/res_pjsip/pjsip_global_headers.c are for the
> independent but related global options issue.  These changes will be
> committed separately.
> 
> res_pjsip: Fixed invalid empty Server and User-Agent SIP headers.
> 
> Setting pjsip.conf useragent to an empty string results in an empty SIP
> header being sent.
> 
> * Made not add an empty SIP header item to the global SIP headers list.
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_pjsip/pjsip_global_headers.c 432666 
>   /branches/13/res/res_pjsip/pjsip_configuration.c 432666 
>   /branches/13/res/res_pjsip/config_global.c 432666 
>   /branches/13/include/asterisk/res_pjsip.h 432666 
> 
> Diff: https://reviewboard.asterisk.org/r/4467/diff/
> 
> 
> Testing
> -------
> 
> Ran through the following pjsip.conf type=global permutations:
> 
> 1) No global object defined.  The defaults are applied with the patch and the Max-Forwards header goes out when it did not before.
> 2) Two global objects defined.  An error message is now output complaining of the multiple global objects.
> 3) One global object defined with custom user_agent value set.  The User-Agent and Server headers have the custom value.
> 4) One global object defined with user_agent value set to an empty string.  The User-Agent and Server headers do not go out when they would go out without a value before.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

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