[asterisk-dev] [Code Review] logger: Call ID logging phase III part 1 split - chan_iax2 and chan_dahdi

[rmudgett]

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


Just concentrated on PRI.

I'm going to leave SS7 alone for now until PRI is complete.


/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1927/#comment12305>

    In this function change the logging functions to explicit callid versions.  We don't need to see the callid information in the SS7 debug logging output.
    
    May need to create a special ast_verbose version to not output callid.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1927/#comment12306>

    In this function change the logging functions to explicit callid versions.  We don't need to see the callid information in the SS7 debug logging output.
    
    For this function it is ast_log_callid and pass NULL for the callid.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1927/#comment12303>

    In this function change the logging functions to explicit callid versions.  We don't need to see the callid information in the PRI debug logging output.
    
    May need to create a special ast_verbose version to not output callid.



/trunk/channels/chan_dahdi.c
<https://reviewboard.asterisk.org/r/1927/#comment12304>

    In this function change the logging functions to explicit callid versions.  We don't need to see the callid information in the PRI debug logging output.
    
    For this function it is ast_log_callid and pass NULL for the callid.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12295>

    Formatting guidelines
    
    Move func_pri_dchannel_new_callid() and func_pri_dchannel_chanpos_callid() to before sig_pri_handle_hold() so you don't need to prototype these static functions.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12298>

    Change to
     * \note Assumes the pri->lock is already obtained.
     * \note Assumes the sig_pri_lock_private(pri->pvts[chanpos]) is already obtained.
    



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12296>

    Formatting guidelines
    



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12301>

    You have not protected against a possible NULL callid pointer.
    
    Also look at how sig_pri_lock_owner is used elsewhere.  sig_pri_lock_owner will return with the owner locked if it is not NULL.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12297>

    Where did this blank line come from?



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12302>

    Blank lines not needed here.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12300>

    Extraneous blank line change.



/trunk/channels/sig_pri.c
<https://reviewboard.asterisk.org/r/1927/#comment12299>

    These routines should be self contained as far as callid is concerned so you don't need to change the function prototypes.  Besides, you have not converted the called functions anyway.


- rmudgett


On June 20, 2012, 4:27 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1927/
> -----------------------------------------------------------
> 
> (Updated June 20, 2012, 4:27 p.m.)
> 
> 
> Review request for Asterisk Developers, rmudgett and Matt Jordan.
> 
> 
> Summary
> -------
> 
> split from: https://reviewboard.asterisk.org/r/1886/
> These changes allow channels dahdi and iax2 to set callids and bind log messages to callids.
> 
> This is the same code as in the earlier review.  It was split to commit the approved parts and to hopefully encourage some review of the remaining parts since there is less in here to review now.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_dahdi.c 369042 
>   /trunk/channels/chan_agent.c 369042 
>   /trunk/channels/chan_iax2.c 369042 
>   /trunk/channels/chan_local.c 369042 
>   /trunk/channels/sig_analog.c 369042 
>   /trunk/channels/sig_pri.c 369042 
>   /trunk/channels/sig_ss7.c 369042 
>   /trunk/main/autoservice.c 369042 
>   /trunk/main/bridging.c 369042 
>   /trunk/main/logger.c 369042 
>   /trunk/main/pbx.c 369042 
> 
> Diff: https://reviewboard.asterisk.org/r/1927/diff
> 
> 
> Testing
> -------
> 
> see https://reviewboard.asterisk.org/r/1886/
> 
> 
> Thanks,
> 
> jrose
> 
>

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