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

[jrose]

> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/main/logger.c, lines 1778-1785
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29176#file29176line1778>
> >
> >     You need to do the same thing here as was done to __ast_verbose().

check.


> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/main/logger.c, lines 1351-1352
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29176#file29176line1351>
> >
> >     Revert this.  I don't think this message is useful.  The callid could be NULL if the earlier call to ast_callid_threadstorage_auto() failed.

check. Failure of ast_callid_threadstorage_auto probably means a rather bad thing happened though... like ran out of memory or something.


> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/channels/sig_ss7.c, line 733
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29171#file29171line733>
> >
> >     The private is also already obtained.
> >     Also this is ss7 not pri.
> >     
> >      * \note Assumes the ss7->lock is already obtained.
> >      * \note Assumes the sig_ss7_lock_private(ss7->pvts[chanpos]) is already obtained.
> >

check.


> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/channels/sig_analog.c, lines 3279-3280
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29169#file29169line3279>
> >
> >     Need a callid autoclean here.

check


> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/channels/sig_analog.c, line 1773
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29169#file29169line1773>
> >
> >     This doesn't need to be initialized.

check


> On June 26, 2012, 1:24 p.m., rmudgett wrote:
> > /trunk/channels/chan_iax2.c, line 1092
> > <https://reviewboard.asterisk.org/r/1927/diff/10/?file=29167#file29167line1092>
> >
> >     This magic number needs a define.  You are using this magic number in several places.
> >     Here and in channel_internal_api.c, and cli.c.

#define AST_CALLID_BUFFER_LENGTH 13

check and check to all of the individual modifications.


- jrose


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


On June 26, 2012, 11:31 a.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1927/
> -----------------------------------------------------------
> 
> (Updated June 26, 2012, 11:31 a.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_agent.c 369392 
>   /trunk/channels/chan_dahdi.c 369392 
>   /trunk/channels/chan_iax2.c 369392 
>   /trunk/channels/chan_local.c 369392 
>   /trunk/channels/sig_analog.c 369392 
>   /trunk/channels/sig_pri.c 369392 
>   /trunk/channels/sig_ss7.c 369392 
>   /trunk/include/asterisk/logger.h 369392 
>   /trunk/main/autoservice.c 369392 
>   /trunk/main/bridging.c 369392 
>   /trunk/main/features.c 369392 
>   /trunk/main/logger.c 369392 
>   /trunk/main/pbx.c 369392 
> 
> 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/20120626/950327b7/attachment-0001.htm>