<p> Attention is currently required from: N A. </p>
<p><a href="https://gerrit.asterisk.org/c/asterisk/+/16075">View Change</a></p><p>14 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/+/16075?tab=comments">Patch Set #9:</a> </p><p style="white-space: pre-wrap; word-wrap: break-word;">Addressing these items will be necessary to get this review merged...<br></p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="null">File funcs/func_lookup.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/+/16075/comment/231e6ee6_bf2d3ed0">Patch Set #9, Line 39:</a> <code style="font-family:monospace,monospace">LOOKUP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Rename to EVAL_EXTEN</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/98614f86_50224a3c">Patch Set #9, Line 41:</a> <code style="font-family:monospace,monospace">Returns the variable-substituted value of an extension in a dialplan context.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Evaluates the contents of a dialplan extension and returns it as a string.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/4af621df_013e218e">Patch Set #9, Line 43:</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;"> <syntax><br> <parameter name="extension" required="true" argsep=","><br> <parameter name="extension" required="true" argsep="@"><br> <argument name="extension" required="true" /><br> <argument name="context" /><br> </parameter><br> <argument name="priority" /><br> </parameter><br> </syntax><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Change to normal [[context,]extensions,]priority syntax.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/590e2f51_a6de8749">Patch Set #9, Line 52:</a> <code style="font-family:monospace,monospace"> <description></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Part of the confusion around this function were the dialplan entries for the data. Rather than <br>exten = _X.,1,<data><br>wrap the data in a Return like so...<br>exten = _X.,1,Return(<data>)</p><p style="white-space: pre-wrap; word-wrap: break-word;">This makes it much clearer what the intent is and it allows the use of Gosub and a possible future CALL_SUB function. In the case of EVAL_EXTEN, the Return isn't actually executed. It also fixes another issue in that ast_get_extension_app() will only return up to the first '(' and ignore the rest. If you use "Return(<data>)" and change the call of ast_get_extension_app() to ast_get_extension_app_data(), you'll get all of <data> even if it contains more parentheses. </p><p style="white-space: pre-wrap; word-wrap: break-word;">Finally provide examples!</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/78407f48_318a36c2">Patch Set #9, Line 53:</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;"> <para>The LOOKUP function returns the parsed value of an extension<br> in the dialplan. Lookup extensions are no different from ordinary<br> extensions, but you may choose to use any arbitrary value or function,<br> similar to usage of the HINT function. Any variables in the lookup value<br> will be resolved in the context of the lookup context. For example,<br> <literal>${EXTEN}</literal> would refer to the LOOKUP extension, not<br> the extension in the context invoking the function.</para><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The EVAL_EXTEN function looks up a dialplan entry by context,extension,priority, evaluates the contents of a Return statement to resolve any variable or function references, and returns the result as a string. you can use this function to create simple user-defined lookup tables or user-defined functions.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Example:<br>[call-types]<br>exten = _1nnn,1,Return(internal)<br>exten = _NXXNXXXXXX,1,Return(external)</p><p style="white-space: pre-wrap; word-wrap: break-word;">[udf]<br>exten = calleridlen,1,Return(${LEN(${CALLERID(num)})})</p><p style="white-space: pre-wrap; word-wrap: break-word;">[default]<br>exten = _X.,1,Verbose(Call type ${EVAL_EXTEN(call-types,${EXTEN},1)})<br>same = n,Verbose(Call type ${EVAL_EXTEN(udf,calleridlen,1)})</p><p style="white-space: pre-wrap; word-wrap: break-word;">Limitations:<br>Because the contents of the extension aren't executed like a Gosub, you can't pass arguments in the EVAL_EXTEN function.</p><p style="white-space: pre-wrap; word-wrap: break-word;">----Of course, format with appropriate XML.----</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/e2f51755_e59bf823">Patch Set #9, Line 62:</a> <code style="font-family:monospace,monospace"> <ref type="function">HINT</ref></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Referencing HINT is confusing. Reference EVAL instead.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/3160a994_71c09250">Patch Set #9, Line 72:</a> <code style="font-family:monospace,monospace"> e = pbx_find_extension(c, NULL, &q, context, exten, priority, NULL, "", E_MATCH);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You need to check if the priority contains non-numeric characters and if it does, you need to pass the priority in the pbx_find_extension "label" parameter and set the match type to E_FINDLABEL. See func_dialplan.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/d9281b41_f7b4661f">Patch Set #9, Line 82:</a> <code style="font-family:monospace,monospace">ast_get_extension_app(e);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This won't work if the value contains any parentheses.<br>exten = 555,1,abc(def)ghi<br>will only return "abc".</p><p style="white-space: pre-wrap; word-wrap: break-word;">Use ast_get_extension_app_data() as noted above.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/f0eed0ab_37e9de8c">Patch Set #9, Line 67:</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;">tatic struct ast_exten *ast_lookup_extension(struct ast_channel *c, const char *context, const char *exten, int priority)<br>{<br> struct ast_exten *e;<br> struct pbx_find_info q = { .stacklen = 0 }; /* the rest is set in pbx_find_context */<br> ast_rdlock_contexts();<br> e = pbx_find_extension(c, NULL, &q, context, exten, priority, NULL, "", E_MATCH);<br> ast_unlock_contexts();<br> return e;<br>}<br><br>static int ast_get_lookup(char *lookup, int lookupsize, char *name, int namesize, struct ast_channel *c, const char *context, const char *exten, int priority)<br>{<br> struct ast_exten *e = ast_lookup_extension(c, context, exten, priority);<br> if (e) {<br> if (name) {<br> const char *tmp = ast_get_extension_app(e);<br> if (tmp)<br> ast_copy_string(name, tmp, namesize);<br> }<br> return 0;<br> }<br> return -1;<br>}<br><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Collapse these two functions.<br>Wrap long lines.<br>Remove the ast_.<br>Remove unused function parameters.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/d62aeb24_5100f517">Patch Set #9, Line 99:</a> <code style="font-family:monospace,monospace"> AST_DECLARE_APP_ARGS(args,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Refactor to use the standard [[context,]extensions,]priority syntax.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/a20fb459_d4468d2e">Patch Set #9, Line 116:</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;">context = exten = args.exten;<br> strsep(&context, "@");<br> if (ast_strlen_zero(context))<br> context = "default";<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Default context should be "this" context and default exten should be "this" extension, like it is in other calls.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/04aa1a6b_17e41ede">Patch Set #9, Line 121:</a> <code style="font-family:monospace,monospace"> priority = atoi(args.priority);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">priority can be a label instead of a number.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/16075/comment/6ecd84bd_8cefa5db">Patch Set #9, Line 127:</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;">/* Substitute variables now, using the location of the lookup */<br> /* strdupa required or we'll just overwrite what we read when we set these */<br> realcontext = ast_strdupa(ast_channel_context(chan));<br> realexten = ast_strdupa(ast_channel_exten(chan));<br> realpriority = ast_channel_priority(chan);<br> ast_channel_lock(chan);<br> ast_channel_context_set(chan, context);<br> ast_channel_exten_set(chan, exten);<br> ast_channel_priority_set(chan, priority);<br> ast_channel_unlock(chan);<br><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't understand this. I thought the objective was to keep the original context, extension and priority, not the lookup target ones?</p><p style="white-space: pre-wrap; word-wrap: break-word;">In any case, any time you call one of the set functions on a channel, the channel snapshot is invalidated which could result in extra AMI and ARI events being generated and potentially cause ARI event listeners to fire.<br>That's not good. If you really want the lookup target's data, you'll have to find another way that doesn't modify the channel.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/16075">change 16075</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/+/16075"/><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: Iad81019689674c9f4ac77d235f5d7234adbb1432 </div>
<div style="display:none"> Gerrit-Change-Number: 16075 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </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: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@sangoma.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sarah Autumn <sarah@endlesstemple.org> </div>
<div style="display:none"> Gerrit-Attention: N A <mail@interlinked.x10host.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 05 Aug 2021 15:42:29 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>