<p> Attention is currently required from: Sean Bright, Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/19898">View Change</a></p><p>4 comments:</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/+/19898?tab=comments">Patch Set #6:</a> </p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I want to do some testing of how the replacement characters are handled, but at a minimum the refactoring of the JSON UTF-8 routines to use the ones in utf8.h should happen in a different review.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Will split.<br>> <br>> Additionally, I don't understand why unrelated UTF-8 functions were modified (switching an 'assert' to an 'if' for example)?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'll revert that. I just noticed that it would cause the dst buffer to be unterminated if size were 0.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/utf8.h:</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/+/19898/comment/9a50072d_a1f955b0">Patch Set #7, Line 136:</a> <code style="font-family:monospace,monospace"> * 3 byte sequence, the destination buffer will beed to be larger than</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">need</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File main/stasis_channels.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/+/19898/comment/fbc516e9_515ce203">Patch Set #7, Line 1168:</a> <code style="font-family:monospace,monospace"> result = ast_utf8_replace_invalid_chars(new_value, &new_value_size,</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">How expensive of an operation is this?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Given that the strings tend to be short and there's no complicated math, not very...<br>I ran the unit tests in a loop and got this...</p><p style="white-space: pre-wrap; word-wrap: break-word;">$ gcc -O2 -o test test.c<br>$ time ./test<br>tests: 181000 pass: 181000 fail: 0</p><p style="white-space: pre-wrap; word-wrap: break-word;">real 0m0.032s<br>user 0m0.032s<br>sys 0m0.001s</p><p style="white-space: pre-wrap; word-wrap: break-word;">On a RaspberryPi 😊</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File tests/test_json.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/+/19898/comment/f8f1c46d_34940bd8">Patch Set #7, Line 1726:</a> <code style="font-family:monospace,monospace"> "app_data", ""</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">This is a behavior change in the internal event, which can be used by things. Is it possible to maintain existing behavior?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">pulling json.c changes to separate review.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19898">change 19898</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/+/19898"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 18 </div>
<div style="display:none"> Gerrit-Change-Id: I4ffbdb19c80bf0efc675d40078a3ca4f85c567d8 </div>
<div style="display:none"> Gerrit-Change-Number: 19898 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: 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: Sean Bright <sean@seanbright.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: Tue, 28 Feb 2023 12:19:10 +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"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>