<p> Attention is currently required from: Mark Murawski. </p>
<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/+/19264">View Change</a></p><p>12 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="null">File channels/chan_dahdi.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/+/19264/comment/8dbac948_8306288c">Patch Set #6, Line 132:</a> <code style="font-family:monospace,monospace">#include "regex.h"</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this supposed to be regex.h or <regex.h>?<br>If it's the latter, it should go above all header files except asterisk.h</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/1c8dca70_66f506db">Patch Set #6, Line 199:</a> <code style="font-family:monospace,monospace">                  busy if BusyOut is enabled.</para></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The description needs to be expanded, namely clarification of what exactly is being answered. When it's enabled, the channel is answered? Or when somebody calls it, their call is answered? (That would set off some alarm bells in people's heads).</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/99ac5a77_06bc4fe1">Patch Set #6, Line 5934:</a> <code style="font-family:monospace,monospace">         AST_APP_ARG(channel);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing is messed up here, and throughout this entire function</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/db58dec6_0b5f08c4">Patch Set #6, Line 8715:</a> <code style="font-family:monospace,monospace">        if (p->busyout) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/29c37964_5155a871">Patch Set #6, Line 9820:</a> <code style="font-family:monospace,monospace">              ast_answer(chan);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If you're really going to answer, you should use ast_raw_answer instead.<br>Have you confirmed this does not work without calling ast_answer? And can you explain why we are answering, as a code comment?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/565d13b9_58184ea7">Patch Set #6, Line 11855:</a> <code style="font-family:monospace,monospace">                       }</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/c8b7eabb_888b59f9">Patch Set #6, Line 15858:</a> <code style="font-family:monospace,monospace">             return 1;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Again, I think returning 1 closes the manager session, this function should always return 0</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/afb6d9e6_77e78f44">Patch Set #6, Line 15880:</a> <code style="font-family:monospace,monospace">              }</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing in this function</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/48dcaf1d_69956e96">Patch Set #6, Line 16561:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/06bff14b_45c06951">Patch Set #6, Line 20335:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">whitespacing</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/ca52a338_cd01e0c9">Patch Set #6, Line 20362:</a> <code style="font-family:monospace,monospace">                usleep(1000);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I wonder if a thread to do this is the most efficient way.<br>Can you not hook into one of the existing DAHDI event loops?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/19264/comment/72283de9_89ae938f">Patch Set #6, Line 20451:</a> <code style="font-family:monospace,monospace">          }</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">else should be on same line</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/19264">change 19264</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/+/19264"/><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: Ib75c87c554bb18b8f65be73b4474474e7e1877b7 </div>
<div style="display:none"> Gerrit-Change-Number: 19264 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: N A <asterisk@phreaknet.org> </div>
<div style="display:none"> Gerrit-Attention: Mark Murawski <markm@intellasoft.net> </div>
<div style="display:none"> Gerrit-Comment-Date: Sat, 14 Jan 2023 14:07:13 +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>