<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/4365/">https://reviewboard.asterisk.org/r/4365/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Besides this minor comment this looks good to me.</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/4365/diff/4/?file=71310#file71310line202" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/cdr/cdr_amqp.c</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">202</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ao2_cleanup</span><span class="p">(</span><span class="n">conf</span><span class="o">-></span><span class="n">global</span><span class="o">-></span><span class="n">amqp</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I don't think this will actually ever be non-NULL. You aren't reusing state so it should get a new instance each time.</pre>
</div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/4365/diff/4/?file=71311#file71311line187" style="color: black; font-weight: bold; text-decoration: underline;">/branches/13/cel/cel_amqp.c</a>
<span style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">187</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ao2_cleanup</span><span class="p">(</span><span class="n">conf</span><span class="o">-></span><span class="n">global</span><span class="o">-></span><span class="n">amqp</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ditto.</pre>
</div>
<br />
<p>- Joshua Colp</p>
<br />
<p>On February 6th, 2015, 1:13 a.m. UTC, David Lee wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By David Lee.</div>
<p style="color: grey;"><i>Updated Feb. 6, 2015, 1:13 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This patch adds an AMQP backend for both CDR and CEL. This allows these
logs to be dispatched to a message broker, such as RabbitMQ, where they
can reliably be queued for transmission to the handling application.
The res_amqp module adds core AMQP protocol support. The provided API
only supports publishing, since that's all that's needed for CDR/CEL.
The AMQP feature of supporting multiple 'channels' per connection was
also omitted. The underlying librabbitmq library is not thread safe, so
the multiplexing benefits are limited. It also makes using the library
more complicated, since clients have to worry about both connections and
channels.
The cdr_amqp module simply registers a CDR backend which encodes CDRs as
JSON, and publishes them to an AMQP connection.
The cel_amqp module similarly registers a CEL backend, which encodes
CELs as JSON, and publishes them to an AMQP connection.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ran amqp-consume to print messages sent to queues.
Confirmed that the amqp test command sent messages properly.
Confirmed that CDRs were properly sent.
Confirmed that CELs were properly sent.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/13/res/res_amqp.exports.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/res/res_amqp.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/res/amqp/internal.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/res/amqp/config.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/res/amqp/cli.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/res/Makefile <span style="color: grey">(431095)</span></li>
<li>/branches/13/makeopts.in <span style="color: grey">(431095)</span></li>
<li>/branches/13/include/asterisk/cel.h <span style="color: grey">(431095)</span></li>
<li>/branches/13/include/asterisk/autoconfig.h.in <span style="color: grey">(431095)</span></li>
<li>/branches/13/include/asterisk/amqp.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/contrib/scripts/install_prereq <span style="color: grey">(431095)</span></li>
<li>/branches/13/configure.ac <span style="color: grey">(431095)</span></li>
<li>/branches/13/configs/samples/cel_amqp.conf.sample <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/configs/samples/cdr_amqp.conf.sample <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/configs/samples/amqp.conf.sample <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/cel/cel_amqp.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/cdr/cdr_amqp.c <span style="color: grey">(PRE-CREATION)</span></li>
<li>/branches/13/build_tools/menuselect-deps.in <span style="color: grey">(431095)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4365/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>