<p>George Joseph <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/9275">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  George Joseph: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">VECTOR: Passing parameters with side effects to macros is dangerous.<br><br>* Fix several instances where we were bumping a ref in the parameter and<br>then unrefing the object if it failed.  The way the AST_VECTOR_APPEND()<br>and AST_VECTOR_REPLACE() macros are implemented means if it fails the new<br>value was never evaluated.<br><br>Change-Id: I2847872a455b11ea7e5b7ce697c0a455a1d0ac9a<br>---<br>M bridges/bridge_softmix.c<br>M res/res_pjsip/pjsip_options.c<br>M res/res_pjsip_history.c<br>M res/res_pjsip_session.c<br>M res/stasis/messaging.c<br>5 files changed, 15 insertions(+), 7 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c<br>index 46b27f1..249985a 100644<br>--- a/bridges/bridge_softmix.c<br>+++ b/bridges/bridge_softmix.c<br>@@ -2085,7 +2085,9 @@<br>           }<br>     }<br> <br>- if (AST_VECTOR_REPLACE(&softmix_data->remb_collectors, bridge_stream_position, ao2_bump(sc->remb_collector))) {<br>+    ao2_ref(sc->remb_collector, +1);<br>+  if (AST_VECTOR_REPLACE(&softmix_data->remb_collectors, bridge_stream_position,<br>+                sc->remb_collector)) {<br>             ao2_ref(sc->remb_collector, -1);<br>   }<br> }<br>diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c<br>index 579f70e..5eaf9e8 100644<br>--- a/res/res_pjsip/pjsip_options.c<br>+++ b/res/res_pjsip/pjsip_options.c<br>@@ -1530,10 +1530,11 @@<br>   ast_debug(3, "Adding endpoint compositor '%s' to AOR '%s'\n",<br>               task_data->endpoint_state_compositor->name, task_data->aor_options->name);<br> <br>+    ao2_ref(task_data->endpoint_state_compositor, +1);<br>         if (AST_VECTOR_APPEND(&task_data->aor_options->compositors,<br>-                ao2_bump(task_data->endpoint_state_compositor))) {<br>+                task_data->endpoint_state_compositor)) {<br>           /* Failed to add so no need to update the endpoint status.  Nothing changed. */<br>-              ao2_cleanup(task_data->endpoint_state_compositor);<br>+                ao2_ref(task_data->endpoint_state_compositor, -1);<br>                 return 0;<br>     }<br> <br>diff --git a/res/res_pjsip_history.c b/res/res_pjsip_history.c<br>index eed06ee..10bcd96 100644<br>--- a/res/res_pjsip_history.c<br>+++ b/res/res_pjsip_history.c<br>@@ -1133,7 +1133,8 @@<br>              } else if (!res) {<br>                    continue;<br>             } else {<br>-                     if (AST_VECTOR_APPEND(output, ao2_bump(entry))) {<br>+                    ao2_bump(entry);<br>+                     if (AST_VECTOR_APPEND(output, entry)) {<br>                               ao2_cleanup(entry);<br>                   }<br>             }<br>diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c<br>index 49ab875..8b1012e 100644<br>--- a/res/res_pjsip_session.c<br>+++ b/res/res_pjsip_session.c<br>@@ -250,7 +250,10 @@<br>          struct ast_sip_session_media *session_media = AST_VECTOR_GET(&media_state->sessions, index);<br>           enum ast_media_type type = ast_stream_get_type(ast_stream_topology_get_stream(cloned->topology, index));<br> <br>-               AST_VECTOR_REPLACE(&cloned->sessions, index, ao2_bump(session_media));<br>+                ao2_bump(session_media);<br>+             if (AST_VECTOR_REPLACE(&cloned->sessions, index, session_media)) {<br>+                    ao2_cleanup(session_media);<br>+          }<br>             if (ast_stream_get_state(ast_stream_topology_get_stream(cloned->topology, index)) != AST_STREAM_STATE_REMOVED &&<br>                   !cloned->default_session[type]) {<br>                  cloned->default_session[type] = session_media;<br>diff --git a/res/stasis/messaging.c b/res/stasis/messaging.c<br>index 77a5874..a7716b8 100644<br>--- a/res/stasis/messaging.c<br>+++ b/res/stasis/messaging.c<br>@@ -457,8 +457,9 @@<br>               ao2_link(endpoint_subscriptions, sub);<br>        } else {<br>              ast_rwlock_wrlock(&tech_subscriptions_lock);<br>-             if (AST_VECTOR_APPEND(&tech_subscriptions, ao2_bump(sub))) {<br>-                     /* Release the ao2_bump that was for the vector and allocation references. */<br>+                ao2_ref(sub, +1);<br>+            if (AST_VECTOR_APPEND(&tech_subscriptions, sub)) {<br>+                       /* Release the refs that were for the vector and the allocation. */<br>                   ao2_ref(sub, -2);<br>                     sub = NULL;<br>           }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/9275">change 9275</a>. To unsubscribe, 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/9275"/><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: merged </div>
<div style="display:none"> Gerrit-Change-Id: I2847872a455b11ea7e5b7ce697c0a455a1d0ac9a </div>
<div style="display:none"> Gerrit-Change-Number: 9275 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>