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

[jrose]

> On May 18, 2012, 5:11 p.m., rmudgett wrote:
> > /trunk/channels/chan_dahdi.c, line 2587
> > <https://reviewboard.asterisk.org/r/1927/diff/1/?file=28057#file28057line2587>
> >
> >     There doesn't seem much need for this function.  It is only used in two places where everywhere else it is done by changing the dahdi_new call directly.
> >     
> >     You have also missed converting several dahdi_new calls.  SS7 and MFC/R2 locations specifically.
> 
> jrose wrote:
>     Nah, this function is actually rather nice.  It lets us return the channel without having to store it in a local variable while still being able to automatically revert the threadstorage afterwards.  After addressing the missed dahdi_new calls it's used three times by the way, so adding all the annoying threadstorage_cleanup and variables that would have to be added to fix this, it's keeping a decent amount of redundant code from being written.

s/redundant code/duplicated code/


- jrose


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


On May 17, 2012, 12:14 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1927/
> -----------------------------------------------------------
> 
> (Updated May 17, 2012, 12:14 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 366844 
>   /trunk/channels/chan_iax2.c 366844 
> 
> 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/20120521/d72b8ab0/attachment-0001.htm>