<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/1576/">https://reviewboard.asterisk.org/r/1576/</a>
</td>
</tr>
</table>
<br />
<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, David Vossel and mjordan.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated Nov. 11, 2011, 2:06 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">Adds back close() in the event that there is a file descriptor with no FILE struct pointer. Pulls the file/descriptor closing operation out of the functions that were doing that and puts it in a new function in tcptls.h
Possibly needs work on the function documentation.</pre>
</td>
</tr>
</table>
<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;">According to the reporter, chan_sip and tcptls were using an odd combination of close and fclose which can result in undefined behavior. Following the man pages for fclose and close, attempting to fclose a file with an already closed file descriptor results in undefined behavior, and fclose itself will already close the file descriptor, so using fclose and then close is redundant. The reporter suggested as well that since file descriptors will experience frequent reuse that using fclose and then close could also result in closing a file descriptor that is in use elsewhere since there is time for that that file descriptor index to be reclaimed in the file descriptor table during the window between that fclose and the following close operation... or at least that's how I interpreted it.
I removed all uses of the close function when there was an adjacent fclose. I'm still not 100% sure if this is the right approach since this behavior introduced in a patch by dvossel in r225445, which can be seen here: http://lists.digium.com/pipermail/asterisk-commits/2009-October/038031.html I'm a little worried that this might be because the file descriptor received a redundant reference somewhere along the line and these close() functions might have been used to close a file descriptor leak or something along those lines.</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;">Set up some TLS calls and used core show fd (with DEBUG_FD_LEAKS enabled) to make sure this wasn't causing a bunch of file descriptor leaks. From what I could find, it wasn't.</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-18700">ASTERISK-18700</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/channels/chan_sip.c <span style="color: grey">(344329)</span></li>
<li>/trunk/include/asterisk/tcptls.h <span style="color: grey">(344329)</span></li>
<li>/trunk/main/tcptls.c <span style="color: grey">(344329)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1576/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>