<p style="white-space: pre-wrap; word-wrap: break-word;">Ready through the code I'm still not sure what an example might look like and why the substitution is being done the way it is. Could you provide an example dialplan of what a call would look like?</p><p>Patch set 1:<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/+/15928">View Change</a></p><p>6 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c">File apps/app_waitforcond.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/+/15928/1/apps/app_waitforcond.c@55">Patch Set #1, Line 55:</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;">                     <parameter name="timeout" /><br>                  <parameter name="interval" /><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Add the unit type for these, and if seconds mention if fractions of a second are supported.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c@82">Patch Set #1, Line 82:</a> <code style="font-family:monospace,monospace">static int waitforcond_exec(struct ast_channel *chan, const char *data)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Some of the failures paths return 0, while others return -1. Should all return -1?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also some failure paths set WAITFORCONDITIONSTATUS to FAILURE while other failure paths do not. Should they all set the status?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c@94">Patch Set #1, Line 94:</a> <code style="font-family:monospace,monospace"> </code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Remove whitespace</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c@107">Patch Set #1, Line 107:</a> <code style="font-family:monospace,monospace"> if (ast_strlen_zero(tmp) || tmp[0] == '\0') {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">The second check is already found in the ast_strlen_zero function so can be removed.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c@113">Patch Set #1, Line 113:</a> <code style="font-family:monospace,monospace"> if (!strchr(tmp, '[') && !strchr(tmp, ']')) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">As you currently have it a string like "ab]cd[ef" would pass your check.</p><p style="white-space: pre-wrap; word-wrap: break-word;">If you want to make this more robust you might want to start the second search for the closing bracket start from the found position of the first search. For example something like:</p><p style="white-space: pre-wrap; word-wrap: break-word;">if (!(open_bracket = strchr(tmp, '[')) && !strchr(open_bracket, ']'))</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15928/1/apps/app_waitforcond.c@203">Patch Set #1, Line 203:</a> <code style="font-family:monospace,monospace">           cond = ast_strdupa(condition);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Do not call strdupa within a loop as it could blow the stack.</p><p style="white-space: pre-wrap; word-wrap: break-word;">You either have to create/free memory on the heap, or you a variable array.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15928">change 15928</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/+/15928"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I08adf2824b8bc63405778cf355963b5005612f41 </div>
<div style="display:none"> Gerrit-Change-Number: 15928 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: N A <mail@interlinked.x10host.com> </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-CC: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 25 May 2021 22:31:07 +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>