[asterisk-dev] [Code Review] 4597: res_pjsip: add CLI commands for global and system configuration

[Mark Michelson]

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



branches/13/res/res_pjsip/config_global.c
<https://reviewboard.asterisk.org/r/4597/#comment25775>

    I think for now this is fine, but there actually is the possibility to misrepresent to the user what is configured here. For instance, if you do not have any global configuration set, then you will have no user_agent option set. This means that Asterisk will add no User-Agent header to outgoing SIP requests at all. However, printing CLI output would show that the user_agent is set to "Asterisk PBX Version 13.3" (or something similar).
    
    When I evaluate this, to me the issue isn't with what you're showing in this CLI command, but rather the somewhat odd behavior associated with how global configuration works. If I have no global configuration at all, then there is no user_agent set. If I have a global configuration section, but I have not set a user_agent, then the user_agent gets set to "Asterisk PBX version 13.3" even though I didn't specify anything.
    
    So rather than raising an issue on this code review, I've opened https://issues.asterisk.org/jira/browse/ASTERISK-24945 to get this inconsistency rectified. When that change gets made, then this check for a NULL cfg can be changed to an assertion instead since it should be impossible to have a NULL global configuration.



branches/13/res/res_pjsip/config_system.c
<https://reviewboard.asterisk.org/r/4597/#comment25774>

    This should be impossible to happen since default system configuration should automatically get set on start-up if no system section was present in pjsip.conf. If default system configuration setup on startup fails, then res_pjsip.so should fail to load.
    
    I think an assertion here that cfg is non-NULL would do the trick instead of trying again to create default configuration settings


- Mark Michelson


On April 7, 2015, 4:05 p.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4597/
> -----------------------------------------------------------
> 
> (Updated April 7, 2015, 4:05 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24918
>     https://issues.asterisk.org/jira/browse/ASTERISK-24918
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Added two new CLI commands for res_pjsip global and system configuration settings:
> 
> pjsip show global
> pjsip show system
> 
> 
> Diffs
> -----
> 
>   branches/13/res/res_pjsip/config_system.c 434150 
>   branches/13/res/res_pjsip/config_global.c 434150 
>   branches/13/CHANGES 434150 
> 
> Diff: https://reviewboard.asterisk.org/r/4597/diff/
> 
> 
> Testing
> -------
> 
> Ran the commands and checked output. Changed some options and reloaded and made sure global settings changed, but system ones did not. Changed some settings again and restarted and made sure both global and system changes too effect. Also removed the sections completely from the pjsip.conf file and made sure the defaults were shown.
> 
> 
> Thanks,
> 
> Kevin Harwell
> 
>

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