<div dir="ltr">On Fri, May 17, 2013 at 2:14 PM, Richard Mudgett <span dir="ltr">&lt;<a href="mailto:rmudgett@digium.com" target="_blank">rmudgett@digium.com</a>&gt;</span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">&gt; I&#39;m working on a fairly invasive set of fixes that apply to 1.8. It<br>
&gt; would be really handy to be able to use RAII_VAR there since I<br>
&gt; intend to submit the same fix for 11 and trunk, as well.<br>
&gt;<br>
&gt;<br>
&gt; Is there any reason we can&#39;t include RAII_VAR in 1.8?<br>
&gt;<br>
&gt; The only reason I can think of is some OS versions have old compilers<br>
&gt; that do not support the gcc feature used for RAII_VAR.<br>
&gt; CentOS comes to mind.<br>
&gt;<br>
&gt;<br>
&gt; It is the greatest thing since sliced bread...<br>
&gt;<br>
&gt; I do know that when we added it, we had at least one bug report from<br>
&gt; someone who was using clang to compile Asterisk. We opted not to<br>
&gt; remove<br>
&gt; RAII_VAR, but at least at the time 11 was new, and they still had 1.8<br>
&gt; to<br>
&gt; use.<br>
&gt;<br>
&gt; Backporting it to 1.8 midstream would be a hard stop to anyone<br>
&gt; attempting to compile Asterisk with anything other than gcc - which<br>
&gt; is,<br>
&gt; admittedly, unsupported.<br>
&gt;<br>
&gt; I don&#39;t think that&#39;s a show-stopper, but it is probably the largest<br>
&gt; risk<br>
&gt; that I can think of. I&#39;m personally fine with it, but at least wanted<br>
&gt; to<br>
&gt; note that before you went ahead and did it.<br>
&gt;<br>
&gt; A better question would be, would you backport it into certified<br>
&gt; asterisk too?<br>
&gt;<br>
&gt; I&#39;m always against backporting to a release branch, simply because we<br>
&gt; never can fully predict the results. That said, I&#39;m still at<br>
&gt; 1.8.7.1, so as long as we expect bug reports to happen and we<br>
&gt; address any regressions with higher priority, but that is just my<br>
&gt; opinion.<br>
&gt;<br>
&gt;<br>
&gt; The size of this backport is tiny. It&#39;s a small macro. It&#39;s just<br>
&gt; incredibly useful and actually helps write code less likely to<br>
&gt; contain errors.<br>
&gt;<br>
&gt;<br>
&gt; If it&#39;s a problem, it shouldn&#39;t be *too* hard to pull it back out.<br>
&gt; The difficultly comes with however many places you have started<br>
&gt; using it. In my case, it&#39;s in the SLA code, so it&#39;s a pretty small<br>
&gt; fraction of the code base. If it gets backported, explodes through<br>
&gt; the 1.8 code, and then a problem is found 6 months later, that would<br>
&gt; be much more painful. I wouldn&#39;t expect it to spread too fast in<br>
&gt; 1.8, since the changes are intentionally minimal. It just might make<br>
&gt; fixing some bugs easier.<br>
&gt;<br>
&gt;<br>
&gt; Compiler version impact is worth considering. I don&#39;t know what the<br>
&gt; specific impact is in terms of distro support, though. How about we<br>
&gt; assume it&#39;s ok, and if there&#39;s an uproar, I agree to do the work to<br>
&gt; revert the usage I added.<br>
<br>
</div></div>You should also backport ao2_cleanup() which was created for use with<br>
RAII_VAR to tolerate NULL pointers.<br>
<br>
void ao2_cleanup(void *obj)<br>
{<br>
  if (obj) {<br>
    ao2_ref(obj, -1);<br>
  }<br>
}<br></blockquote><div><br></div><div style>Good call.  Thanks.</div><div style><br></div><div style>I had a temporary equivalent helper in app_meetme.c for now, but I guess it makes sense to backport the general one.</div>

<div style><br></div><div style>-- </div><div style>Russell Bryant </div></div></div></div>