<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/3666/">https://reviewboard.asterisk.org/r/3666/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 23rd, 2014, 4:36 p.m. CDT, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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/3666/diff/1/?file=60383#file60383line4084" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/res/res_fax.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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">4084</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">filenames</span> <span class="o">=</span> <span class="n">generate_filenames_string</span><span class="p">(</span><span class="n">session</span><span class="o">-></span><span class="n">details</span><span class="p">,</span> <span class="s">""</span><span class="p">,</span> <span class="s">", "</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This call can fail. If it does, emit an error and bail appropriately.</pre>
</blockquote>
<p>On June 23rd, 2014, 6:17 p.m. CDT, <b>Jonathan Rose</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Problem is, there is currently no way to detect whether generate_filenames_string returned NULL due to an empty list or if it was actually due to an allocation failure. Currently the CLI output doesn't try to distinguish between the two, but I suppose at the very least I should add some NULL checking.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Nevermind, I already had NULL checking for the event. I'll go ahead and repost the review. If we want to pursue this further, I'll need to change how generate_filenames_string works.</pre>
<br />
<p>- Jonathan</p>
<br />
<p>On June 23rd, 2014, 2:42 p.m. CDT, Jonathan Rose 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, Matt Jordan and Mark Michelson.</div>
<div>By Jonathan Rose.</div>
<p style="color: grey;"><i>Updated June 23, 2014, 2:42 p.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;">More CLI to AMI command conversions, this time focusing on everyone's favorite method for sending documents, FAX!
FAXSessions replicates the functionality of fax show sessions, and is more or less a 1:1 duplication.
FAXSession replicates the functionality of fax show session. Output is slightly stripped down from the CLI variant in order to keep things consistent across multiple FAX modules and not just spandsp.
FAXStats replicates the functionality of fax show stats, but only provides the fields that res_fax would normally provide in fax show stats and not any of the technology specific fields.</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;">Created some simple transmit/receive fax sessions by originating calls to/from faxsend/faxreceive extensions and then ran each of these commands.
Did the above with and without action_id included to make sure it would be reproduced across events and responses.
Checked the output of documentation for events and actions for sanity.</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>/trunk/res/res_fax_spandsp.c <span style="color: grey">(416868)</span></li>
<li>/trunk/res/res_fax.c <span style="color: grey">(416868)</span></li>
<li>/trunk/include/asterisk/res_fax.h <span style="color: grey">(416868)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3666/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>