<p style="white-space: pre-wrap; word-wrap: break-word;">The current approach seems incorrect. A vector is not an ao2_object, so you won't be able to use ao2 type functions on it. Do you not want the vector to potentially change while accessing it. IF so then I think AST_VECTOR_RW will be better to use here, and lock where appropriate.</p><p>Patch set 2:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14684">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c">File res/res_musiconhold.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/asterisk/+/14684/2/res/res_musiconhold.c@334">Patch Set #2, Line 334:</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;">   ao2_lock(state->class);<br>    files = ao2_bump(state->class->files);<br>  ao2_unlock(state->class);<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">state->class-files is a vector, and not an ao2 object. I'd expect the "bump" to assert.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c@348">Patch Set #2, Line 348:</a> <code style="font-family:monospace,monospace">   } else if (state->save_pos >= 0 && state->save_pos < file_count && !strcmp(AST_VECTOR_GET(files, state->save_pos), state->save_pos_filename)) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">A reference of the vector still won't protect r/w conflicts. I think you need to make the vector an AST_VECTOR_RW?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14684/2/res/res_musiconhold.c@403">Patch Set #2, Line 403:</a> <code style="font-family:monospace,monospace">    ao2_ref(files, -1);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">files is not an ao2_object so this will assert or cause problems.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14684">change 14684</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/c/asterisk/+/14684"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-Change-Id: I479c5dcf88db670956e8cac177b5826c986b0217 </div>
<div style="display:none"> Gerrit-Change-Number: 14684 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 27 Jul 2020 21:04:14 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>