[asterisk-dev] [Code Review] Change a few warnings to debug and the inverse (pbx, sip, rtp, cdr_odbc)

wdoekes reviewboard at asterisk.org
Wed Oct 17 04:45:48 CDT 2012


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



/branches/1.8/cdr/cdr_odbc.c
<https://reviewboard.asterisk.org/r/2167/#comment14064>

    This deserves a warning (or perhaps an error even). See ASTERISK-20538.



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2167/#comment14065>

    I was hesitant about changing this one. SDP without audio is somewhat uncommon.
    
    I decided that consistency is better, so I changed it to the same debug level as the 0-port video warning.



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2167/#comment14066>

    Leif agrees with me that this should not be a warning:
    http://lists.digium.com/pipermail/asterisk-dev/2012-October/057293.html



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2167/#comment14067>

    Perhaps I should change it to 2+, since the same debug message is for verbosity 2+ a bit lower down.



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/2167/#comment14068>

    I don't need a warning if someone else offers to set up srtp and I don't have it.
    
    When *I* want to initiate srtp, I get extra warnings anyway:
    
      ast_log(LOG_WARNING, "SRTP audio setup failed\n");
    
    Then I can increase debug to debug why.



/branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/2167/#comment14069>

    Comma already works in 1.4.
    This warning got added in 1.6 somewhere.
    
    I get that warning when people have a pipe between their name and their company name (e.g. "Walter | OSSO") when I do something as simple as this:
    
      NoOp(display_name = "${CALLERID(name)}")
    
    So I'd like to get rid of it.
    
    But if you say we should remove this first in 10 or 11 or trunk, that'd be fine with me too. (Personally I'd suggest 11, but I didn't want to create a separate review for it.)



/branches/1.8/res/res_rtp_asterisk.c
<https://reviewboard.asterisk.org/r/2167/#comment14070>

    We're talking (non-standard?) RTP keepalives here.
    
    This is not something that needs a warning.
    
    See this:
    http://wjd.nu/files/2012/10/read-too-short-rtp-keepalive.png?view
    
    If you like, I can drop the memcmp and reduce the WARNING to a DEBUG instead?


- wdoekes


On Oct. 17, 2012, 4:18 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2167/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2012, 4:18 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> I've changed a couple of ast_log's to ast_debug, and some ast_verb to ast_log. See the to-be-created review for annotations/why.
> 
> 
> This addresses bug ASTERISK-20538.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20538
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/cdr/cdr_odbc.c 375136 
>   /branches/1.8/channels/chan_sip.c 375136 
>   /branches/1.8/main/pbx.c 375136 
>   /branches/1.8/res/res_rtp_asterisk.c 375136 
> 
> Diff: https://reviewboard.asterisk.org/r/2167/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20121017/b77f5107/attachment-0001.htm>


More information about the asterisk-dev mailing list