<p> Attention is currently required from: Sean Bright, N A, Joshua Colp. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/18829">View Change</a></p><p>15 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">Commit Message:</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/+/18829/comment/1829eb6b_ac519136">Patch Set #14, Line 20:</a> <code style="font-family:monospace,monospace"> same => n,Verbose(test_sentence=${REDIS(test_sentence)})</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The commit message is not the right place for an example. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Ack</p></li></ul></li><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/+/18829?tab=comments">Patch Set #15:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Thanks</p><p style="white-space: pre-wrap; word-wrap: break-word;">Fixed most.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File configs/samples/hiredis.conf.sample:</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/+/18829/comment/48f8a09a_901a14f1">Patch Set #12, Line 3:</a> <code style="font-family:monospace,monospace">;server = 127.0.0.1 ; server[:port] of Redis server to use, default=127.0.0.1.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Does this support IPv6 as well? If so, it should be stated</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">hiredis 0.12.0 (2015-01-22) added IPV6 support.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Debian buster hiredis 0.14.0-3, which is now considered old.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File include/asterisk/autoconfig.h.in:</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/+/18829/comment/7c415cb4_9cc1d2c1">Patch Set #14, Line 29:</a> <code style="font-family:monospace,monospace">/* Define to 1 if using 'alloca.c'. */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Did you mean to remove CRAY_STACKSEG_END? Or is this a possible rebase issue?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I now don't use alloca.c</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/5497e04d_bb1e3bbb">Patch Set #14, Line 504:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same for this, why is memory. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">probably alloca not used anymore</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File res/res_hiredis.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/+/18829/comment/4d46c26e_fe7e6328">Patch Set #14, Line 49:</a> <code style="font-family:monospace,monospace"> <syntax argsep=""></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why is the argsep empty? If it's not relevant, then remove the argsep attribute.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/cfe2ecff_5a01a1a2">Patch Set #14, Line 50:</a> <code style="font-family:monospace,monospace"> <parameter name="valid redis syntax" required="true" /></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I would use underscores instead of spaces, if only to adhere to how all other modules do it.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/de6c53b5_91246860">Patch Set #14, Line 53:</a> <code style="font-family:monospace,monospace"> <para>This function will pass without validation any Redis syntax to a Redis database</para></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">add a comma before and after "without validation"</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/0186c67f_501dd9d2">Patch Set #14, Line 79:</a> <code style="font-family:monospace,monospace"> If you wish to find out if an entry exists, use a REDIS_EXISTS</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think "If you wish..." should be a separate <para> for readability.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/83263ac8_9cc64dbd">Patch Set #14, Line 121:</a> <code style="font-family:monospace,monospace"> <ref type="function">REDIS</ref></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I noticed you only have see also for the REDIS function for several of these functions. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/9fc37500_5ed7c834">Patch Set #14, Line 152:</a> <code style="font-family:monospace,monospace"> <synopsis>Background Save on module unload</synopsis></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The synopsis for this option is a little unclear and could be expanded/clarified</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/32dc12e2_c7d39211">Patch Set #14, Line 211:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Extra line</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/d78f9a13_c9ce2b47">Patch Set #14, Line 305:</a> <code style="font-family:monospace,monospace">static int __attribute__((format(printf, 3, 0))) hiredis_command(char* buf, size_t len, const char* format, ...) </code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">redness</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/da37c149_b1c2fa44">Patch Set #14, Line 638:</a> <code style="font-family:monospace,monospace"> * even if live_dangerously is disabled.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I thought DB writing requires live dangerously be enabled. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I copied func_db.c for these function, so will have to look deeper.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/18829/comment/516ce2cc_63964b83">Patch Set #14, Line 767:</a> <code style="font-family:monospace,monospace"> ast_debug(6, " password = %s\n", cfg->general->password);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think some debugs are fine but writing the password into the log on init might be a bit too much. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">agreed</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/18829">change 18829</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/+/18829"/><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: Ifdf4d33d2a2a5fd044fac13b201c7437de34ba6a </div>
<div style="display:none"> Gerrit-Change-Number: 18829 </div>
<div style="display:none"> Gerrit-PatchSet: 15 </div>
<div style="display:none"> Gerrit-Owner: Alec Davis <alec@bdt.co.nz> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: N A <mail@interlinked.x10host.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: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Attention: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 29 Jul 2022 13:16:21 +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: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Comment-In-Reply-To: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>