<p><a href="https://gerrit.asterisk.org/10747">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/10747/2/main/cdr.c">File main/cdr.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/10747/2/main/cdr.c@1627">Patch Set #2, Line 1627:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">   if ((!cdr->start.tv_sec) && (!cdr->start.tv_usec)) {<br>            cdr->start = ast_tvnow();<br>  }<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Yes, I was thinking about that too. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think any modifications to CDRs are acceptable without understanding the consequences of it and detailing it, even if the code fix is small. It is possible that while this fixes your problem it then breaks another. That's what I was referring to with the state machine and the scenario.</p><p style="white-space: pre-wrap; word-wrap: break-word;">It's not a question of "should the start time be reset?" for your scenario, it's a question of "this explicitly reset it before, so was that on purpose? what was that scenario? was that correct?". The answer may indeed be your fix in the end but without a more concrete understanding I'm not comfortable with the change as it could fix one thing, and break another.</p><p style="white-space: pre-wrap; word-wrap: break-word;">What I'd like to see is a comment before the code stating the scenarios where the start time can already be set, and why that is acceptable.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/10747">change 10747</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/10747"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I921bc04064b6cff1deb2eea56a94d86489561cdc </div>
<div style="display:none"> Gerrit-Change-Number: 10747 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 (1000185) </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: sungtae kim <pchero21@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 06 Dec 2018 12:48:03 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>