[Asterisk-code-review] func_lookup: Fork of HINT function (asterisk[master])

[N A]

Attention is currently required from: George Joseph.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/16075 )

Change subject: func_lookup: Fork of HINT function
......................................................................


Patch Set 9:

(1 comment)

File funcs/func_lookup.c:

https://gerrit.asterisk.org/c/asterisk/+/16075/comment/a64de643_6386d7a5 
PS9, Line 127: /* Substitute variables now, using the location of the lookup */
             : 	/* strdupa required or we'll just overwrite what we read when we set these */
             : 	realcontext = ast_strdupa(ast_channel_context(chan));
             : 	realexten = ast_strdupa(ast_channel_exten(chan));
             : 	realpriority = ast_channel_priority(chan);
             : 	ast_channel_lock(chan);
             : 	ast_channel_context_set(chan, context);
             : 	ast_channel_exten_set(chan, exten);
             : 	ast_channel_priority_set(chan, priority);
             : 	ast_channel_unlock(chan);
             : 
> I don't understand this. […]
Correct... with a slight technicality.

The reason I added this is that variable substitution doesn't work properly with EXTEN (and also CONTEXT and PRIORITY, though use cases for this are probably rare or non-existent). Without this, EXTEN is the exten of the calling extension. The actual intent would be for EXTEN to be evaluated as the EXTEN of the lookup.

e.g.
[lookup]
exten => something,2,${EXTEN:-1}

[parent]
exten => s,1,NoOp(${EVAL_EXTEN(lookup,something,2)})

We would expect that the function returns: g

Without the code referenced, it will instead return: s.

I couldn't think of a better way to do it at the time. People will expect EXTEN in the lookup context to be the exten there, not the calling exten, and rightly so. Do you have any thoughts or ideas on this? Proper EXTEN evaluation is kind of an important thing. We don't actually need to change the extension, we just need the variable substituter to parse it properly.

Other than this thought, everything sounds good and I will work on this over the next few days.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/16075
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Iad81019689674c9f4ac77d235f5d7234adbb1432
Gerrit-Change-Number: 16075
Gerrit-PatchSet: 9
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Sarah Autumn <sarah at endlesstemple.org>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Thu, 05 Aug 2021 15:51:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: George Joseph <gjoseph at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210805/d9dc6690/attachment-0001.html>