<p> Attention is currently required from: Sean Bright, Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16640">View Change</a></p><p>1 comment:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Patchset:</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/+/16640?tab=comments">Patch Set #3:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can you please tell me under what circumstances this has actually been a problem? What invocation of […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Hi Sean,</p><p style="white-space: pre-wrap; word-wrap: break-word;">As indicated on IRC, external (out of tree) C++ modules.  As discussed there are a few more potential improvements that can be made to this patch, but as it stands, the "worse code path" will only affect the case where the buffer gets overrun.</p><p style="white-space: pre-wrap; word-wrap: break-word;">We've had that in one of our own modules with a fairly mundane function name, but with a somewhat large number of arguments.  Simply making the buffer larger merely delays the problem, making it dynamic worsens the code path in all cases (and just results in nearly unreadable logs anyway, so limiting the buffer does make sense).</p><p style="white-space: pre-wrap; word-wrap: break-word;">The primary concept I think I may be able to utilize from your patch is to just use sprintf once, even for the "bad" path, whereas this patch calls it twice.  I suspect the best possible option will be to kind-of go back to my original patch, use snprintf to push in the initial escape sequence (at this point we can already determine if more than the escape sequences will fit), a second snprintf for the input buffer (so we can get the string length in the same step which strcpy won't give is requiring an extra strlen), and then truncate as required before putting in the trail.  But since we can then determine before the second snprintf the original patch (set 2) is pretty much equivalent in that case.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16640">change 16640</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/+/16640"/><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: I80ef7a4bfd2947e090ef830143391d11baebdb0d </div>
<div style="display:none"> Gerrit-Change-Number: 16640 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Jaco Kroon <jaco@uls.co.za> </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-Reviewer: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-CC: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Attention: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Sat, 13 Nov 2021 19:46:26 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: Sean Bright <sean@seanbright.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>