[asterisk-dev] [Code Review] 2900: res_pjsip_logger: Add CLI command to disable logger or enable it globally or for an IP address

[Mark Michelson]

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


Most critical thoughts I have while going through this are in the realm of "Ooh, we could do X instead and it could be better!" But for the sake of getting something in and working, what's here works.

There is one biggish change I would recommend here. Instead of using the word "ip" in the CLI command, I would recommend using something like "host" instead. The reason is that "ip" implies the restriction of accepting just an IP address. In fact, you can put an IP address, or an IP address and port, or an FQDN, and the command will work.


/branches/12/res/res_pjsip_logger.c
<https://reviewboard.asterisk.org/r/2900/#comment19080>

    I know this was copied from chan_sip, but for this, change the final 0 to AST_AF_UNSPEC.



/branches/12/res/res_pjsip_logger.c
<https://reviewboard.asterisk.org/r/2900/#comment19081>

    Use ast_sockaddr_setnull() here.


- Mark Michelson


On Oct. 2, 2013, 8:59 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2900/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 8:59 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> People might not appreciate that chan_pjsip's logger can only be turned on and off by means of loading and unloading the module.  This provides some of the functionality of sip set debug. Currently supported:
> 
> pjsip set logger on
> pjsip set logger off
> pjsip set logger ip <ip with or without port>
> 
> Also changed the tags so that the logger is installed by default (but will be off until you set the logger on)
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_logger.c 400284 
> 
> Diff: https://reviewboard.asterisk.org/r/2900/diff/
> 
> 
> Testing
> -------
> 
> Tested two endpoints to see if sip traffic came in when the logger was off.  It didn't.
> Tested them both when logger was set on globally. It came in for both.
> Tested that setting IP mode for a single IP address resulted in messages only happening for the endpoint with that IP address. Confirmed.
> Tested that setting port in the address to something other than 5060 would result in no messages. Confirmed.
> Tested that setting port in the address to 5060 would make the messages come in. Confirmed.
> 
> 
> Thanks,
> 
> jrose
> 
>

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