<p>Patch set 6:<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/+/14339">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/14339/6/res/res_stir_shaken.c">File res/res_stir_shaken.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/+/14339/6/res/res_stir_shaken.c@295">Patch Set #6, Line 295:</a> <code style="font-family:monospace,monospace"> ast_log(LOG_ERROR, "Failed to allocate space for STIR/SHAKEN datastore "</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">the datastore needs to be freed in this path too.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/14339/6/res/res_stir_shaken.c@1171">Patch Set #6, Line 1171:</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;"> ast_channel_unlock(chan);<br> if (current_index != target_index || !datastore) {<br> ast_log(LOG_WARNING, "No STIR/SHAKEN results for index '%s'\n", args.first_param);<br> return -1;<br> }<br> ss_datastore = datastore->data;<br><br> if (!strcasecmp(args.second_param, "identity")) {<br> ast_copy_string(buf, ss_datastore->identity, len);<br> } else if (!strcasecmp(args.second_param, "attestation")) {<br> ast_copy_string(buf, ss_datastore->attestation, len);<br> } else if (!strcasecmp(args.second_param, "verify_result")) {<br> ast_copy_string(buf, stir_shaken_verification_result_to_string(ss_datastore->verify_result), len);<br> } else {<br> ast_log(LOG_ERROR, "No such value '%s' for %s\n", args.second_param, function);<br> return -1;<br> }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Does this also need to be protected by the channel lock? seems like if the channel goes away, and the associated datastore this could be a problem.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/14339">change 14339</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/+/14339"/><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-Change-Id: Ice6d52a3a7d6e4607c9c35b28a1f7c25f5284a82 </div>
<div style="display:none"> Gerrit-Change-Number: 14339 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 11 May 2020 22:35:57 +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>