<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/1270/">https://reviewboard.asterisk.org/r/1270/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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, Russell Bryant and David Vossel.</div>
<div>By jrose.</div>
<p style="color: grey;"><i>Updated 2011-06-15 09:23:07.988540</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;">This patch runs PQclear a number of times on PGresult objects in situations where functions previously would have returned without clearing the PGresult object. Also, some PQclear statements that were already in the code have been moved to occur before the result is unlocked. I don't know if this is a necessary change since presumably at that point the PGresult isn't going to be used anymore anyway, but I don't see it as likely to hurt anything either. I suppose if it doesn't really help anything though, we could undo that change since it would just be adding one more command of holding a lock.
The reporter seems to have worked in this area a good bit.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing (updated)</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;">Testing was fairly rudimentary. I applied the patch (by hand because the reporter's submission wouldn't apply. For some reason.), switched my extconfig.conf to use pgsql for the connection and set res_pgsql.conf for my pgsql database and user and made sure my realtime peers still worked without being broken. It didn't break under the stuff I put it through and all of these changes seem rather tame. But locking is involved, so I decided to go ahead and wait for review before committing it, especially since it's not a critical issue.
My testing was mostly based on realtime peers and involved some bad database changes and some basic calling/registering to make sure nothing exploded. I'll keep testing this throughout the rest of the day and a little tomorrow morning and report back if I find anything wrong.
Update: Tested some of the patch changes using verb statements to trace exit conditions. Couldn't get to some of the changes, but they are generally all the same and the ones I did reach were all fine (no crashes, no strange behavior). Still don't know if it adds anything of immediate value, but it does change stuff to be more consistent.</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/19245">19245</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/res/res_config_pgsql.c <span style="color: grey">(323454)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1270/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>