-----------------------------------------------------------
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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to