<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/1996/">https://reviewboard.asterisk.org/r/1996/</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;">This looks like the right way to solve this problem.</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/1996/diff/2/?file=29099#file29099line296" style="color: black; font-weight: bold; text-decoration: underline;">/branches/1.8/main/cdr.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void ast_cdr_getvar(struct ast_cdr *cdr, const char *name, char **ret, char *workspace, int workspacelen, int recur, int raw)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">296</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">name</span><span class="p">,</span> <span class="s">"duration"</span><span class="p">))</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">296</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">name</span><span class="p">,</span> <span class="s">"duration"</span><span class="p">))</span><span class="hl"> </span><span class="p"><span class="hl">{</span></span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">297</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">snprintf</span><span class="p">(</span><span class="n">workspace</span><span class="p">,</span> <span class="n">workspacelen</span><span class="p">,</span> <span class="s">"%ld"</span><span class="p">,</span> <span class="n">cdr</span><span class="o">-></span><span class="n">duration</span> <span class="o">?</span> <span class="n">cdr</span><span class="o">-></span><span class="n">duration</span> <span class="o">:</span> <span class="p">(</span><span class="kt">long</span><span class="p">)</span><span class="n">ast_tvdiff_ms</span><span class="p">(</span><span class="n">ast_tvnow</span><span class="p">(),</span> <span class="n">cdr</span><span class="o">-></span><span class="n">start</span><span class="p">)</span> <span class="o">/</span> <span class="mi">1000</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">297</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">snprintf</span><span class="p">(</span><span class="n">workspace</span><span class="p">,</span> <span class="n">workspacelen</span><span class="p">,</span> <span class="s">"%ld"</span><span class="p">,</span> <span class="n">cdr</span><span class="o">-></span><span class="n">duration</span><span class="hl"> </span><span class="o"><span class="hl">||</span></span><span class="hl"> </span><span class="o"><span class="hl">!</span></span><span class="n"><span class="hl">ast_tvzero</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">cdr</span></span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">end</span></span><span class="p"><span class="hl">)</span></span> <span class="o">?</span> <span class="n">cdr</span><span class="o">-></span><span class="n">duration</span> <span class="o">:</span> <span class="p">(</span><span class="kt">long</span><span class="p">)</span><span class="n">ast_tvdiff_ms</span><span class="p">(</span><span class="n">ast_tvnow</span><span class="p">(),</span> <span class="n">cdr</span><span class="o">-></span><span class="n">start</span><span class="p">)</span> <span class="o">/</span> <span class="mi">1000</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">298</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">name</span><span class="p">,</span> <span class="s">"billsec"</span><span class="p">))</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">298</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl"> </span><span class="p"><span class="hl">}</span></span><span class="hl"> </span><span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">strcasecmp</span><span class="p">(</span><span class="n">name</span><span class="p">,</span> <span class="s">"billsec"</span><span class="p">))</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">299</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">snprintf</span><span class="p">(</span><span class="n">workspace</span><span class="p">,</span> <span class="n">workspacelen</span><span class="p">,</span> <span class="s">"%ld"</span><span class="p">,</span> <span class="n">cdr</span><span class="o">-></span><span class="n">billsec</span> <span class="o">||</span> <span class="n">cdr</span><span class="o">-></span><span class="n">answer</span><span class="p">.</span><span class="n">tv_sec</span> <span class="o">==</span> <span class="mi">0</span> <span class="o">?</span> <span class="n">cdr</span><span class="o">-></span><span class="n">billsec</span> <span class="o">:</span> <span class="p">(</span><span class="kt">long</span><span class="p">)</span><span class="n">ast_tvdiff_ms</span><span class="p">(</span><span class="n">ast_tvnow</span><span class="p">(),</span> <span class="n">cdr</span><span class="o">-></span><span class="n">answer</span><span class="p">)</span> <span class="o">/</span> <span class="mi">1000</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">299</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">snprintf</span><span class="p">(</span><span class="n">workspace</span><span class="p">,</span> <span class="n">workspacelen</span><span class="p">,</span> <span class="s">"%ld"</span><span class="p">,</span> <span class="n">cdr</span><span class="o">-></span><span class="n">billsec</span> <span class="o">||</span> <span class="n">cdr</span><span class="o">-></span><span class="n">answer</span><span class="p">.</span><span class="n">tv_sec</span> <span class="o">==</span> <span class="mi">0</span> <span class="o">?</span> <span class="n">cdr</span><span class="o">-></span><span class="n">billsec</span> <span class="o">:</span> <span class="p">(</span><span class="kt">long</span><span class="p">)</span><span class="n">ast_tvdiff_ms</span><span class="p">(</span><span class="n">ast_tvnow</span><span class="p">(),</span> <span class="n">cdr</span><span class="o">-></span><span class="n">answer</span><span class="p">)</span> <span class="o">/</span> <span class="mi">1000</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;">You could, as an optimization, cut the comparison down to just checking whether cdr->end.tv_sec != 0, as is done below. Also, in all cases where cdr->end is zero, duration will always be zero. So the first check is superfluous.</pre>
</div>
<br />
<p>- Tilghman</p>
<br />
<p>On June 21st, 2012, 11:08 a.m., Matt Jordan wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers, Mark Michelson and Tilghman Lesher.</div>
<div>By Matt Jordan.</div>
<p style="color: grey;"><i>Updated June 21, 2012, 11:08 a.m.</i></p>
<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;">Certain places in core/cdr.c would, if the duration value were 0, would calculate the duration as being the delta between the current time and the time at which the CDR record was started. While this does not typically cause a problem in non-batch mode, this can cause an issue in batch mode where CDR records are gathered and written long after those calls have ended. In particular, this affects calls that were never answered, as those are expected to have a duration of 0. Often, this would result in CDR logs with a significant number of calls with lengthy durations, but dispositions of "BUSY".
Note that this does not affect CSV CDRs, as that backend does not use ast_cdr_getvar and instead directly reports the duration value. The affected core backends include cdr_apative_odbc and cdr_custom; other extended or deprecated CDR backends may potentially still directly manipulate the duration values.
This patch essentially reverts r91617, when this behavior was introduced.</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 through the Asterisk Test Suite's CDR tests. A new test for batch CDR creation was also written that also tests that the disposition/billsec are 0 when they should be, and non-zero when the calls are answered.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-19860">ASTERISK-19860</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/main/cdr.c <span style="color: grey">(369001)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1996/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>