Summary:ASTERISK-01804: [post-1.0][patch] Adds the ability to copy a channel variable from any channel to the current channel
Reporter:hwstar (hwstar)Labels:
Date Opened:2004-06-11 22:14:26Date Closed:2004-11-17 23:17:09.000-0600
Versions:Frequency of
Environment:Attachments:( 0) getchanvars.patch
( 1) new-var-functions.tar.gz
Description:This patch allows any channel to get a copy of a channel variable from an arbitrary channel which is active. All
channel variables are accessible including the builtin ones

Source files affected: pbx.c

The need for this was borne out of getting the caller ID information from a primary logical zap channel while the
user was on the secondary logical zap channel.


Example of use:

Comments:By: Mark Spencer (markster) 2004-06-12 09:47:51

There should probably be a way to know if you were unable to find the referenced channel (e.g. exiting to n + 101 if it doesn't exist)?

Also, if you're breaking things down into builtin_chanvars, why not just make a convenience routine that gives you any variable for a given channel rather than doing all that processing within the app itself?  Then that function might be used by things like AGI, so they have access to builtin vars like ${EXTEN}.

By: Mark Spencer (markster) 2004-06-12 09:48:32

Actually, is there any reason we shouldn't just extend pbx_builtin_getvar_helper to have that functionality that occurs to you?

By: hwstar (hwstar) 2004-06-12 13:01:44


I thought that I might break someone's dialplan with the special treatment of '@' if I included it in pbx_builtin_getvar_helper. If you think that the user's dialplans won't get screwed up using this nomenclature, then, it would be better to include this functionality in function useable by everyone else. Is there a better character to use than '@' in your opinion?

As far as jumping to n + 101, I have comments from others (JT) that this is a bad idea from a user standpoint. If you want to go with the changes to pbx_builtin_getvar_helper, it would also be a bad idea to change how it works.

It will be trivial to change the code to access the channel builtins from pbx_builtin_getvar_helper. Is this what you want to do?


By: hwstar (hwstar) 2004-06-12 21:24:30

Also, should pbx_substitute_variables_temp be changed to provide '@' functionallity for extensions and expressions?


By: hwstar (hwstar) 2004-06-13 18:17:44

After analyzing the code I found a shortcoming in the interface of pbx_builtin_getvar_helper. Ther way it's currently coded, it returns a pointer to an item in the varible list, which is ok when the data storage is pre-allocated. For the builtin channel variables, however, the values are built on the fly and there's no thread-safe way that I know of that will allow a pointer to be returned to a valid memory area, when the storage for th value is allocated in the called function. In other words, Everything goes out of scope when pbx_builtin_getvar_helper returns leaving you a pointer to undefined memory.

What would have been a better interface? How about having the caller pass 2 additonal args:a pointer to some storage space, and the length of that storage space.

It's probably too much hassle at this point to change the interface of pbx_builtin_getvar_helper, but a new helper function could be defined.
Any thoughts on this?


By: Mark Spencer (markster) 2004-06-13 18:24:05

My idea was to make getvar_helper know about builtin variables, not the "@" syntax.  The "@" syntax would have reentrancy issues potentially since getvar_helper is often called with chan->lock held, and will be trying to grab another chan->lock, potentially leading to a deadlock if we aren't careful.

By: hwstar (hwstar) 2004-06-15 11:20:19


Even if the @ syntax is removed and I only implement the builtin variable feature, there still remains the problem of having a per-thread allocated storage area for the builtin's value. The current implementation pbx_builtin_getvar_helper function function just returns a pointer to the actual location where the non-builtin variable is stored in the varshead linked list.

In the case of the builtin variables, the storage isn't pre-allocated, this means that either a per-channel linked list of builtin variables has to be created at initialization time, or the caller to pbx_builtin_getvar_helper has to supply a pointer and a length to a storage area
for the return value. Which is best?


By: Mark Spencer (markster) 2004-06-18 09:21:46

Hrm...  Good question.  I don't really like the name "pbx_builtin_getvar_helper" anyway....

Lets just create:

/* Set a variable */
int ast_variable_set(struct ast_channel *chan, const char *variable, const char *value);

/* Return variable, stored in buffer space provided */
char *ast_variable_get(struct ast_channel *chan, const char *variable, char *value, int maxlen);

/* Return "strdup'd" version of value */
char *ast_variable_get_alloc(struct ast_channel *chan, const char *variable);

And we'll leave the pbx_builtin_getvar_helper and pbx_bulitin_setvar_helper for the applications that truly need it.  The risk is that if you do pbx_builtin_getvar_helper on a channel you don't own, then there is a risk the channel will disappear before you look at the value.

By: hwstar (hwstar) 2004-06-19 22:32:00


Sounds good to me. If we leave pbx_builtin_getvar_helper as is, it will not be able to return the builtin channel variables because of the problem I mentioned in my previous message. If this is not what you had in mind, add a bugnote. Otherwise, I'll make these changes, and hopefully upload a new patch in the next few days.


By: Mark Spencer (markster) 2004-06-19 23:58:56

It's fine just as it is.  We'll have to find the places that are appropriate to change to the new functions.

By: hwstar (hwstar) 2004-06-20 22:51:17


Do you think it's too dangerous to have ast_variable_set set builtin channel
variables, or was this what you had in mind? Also, we ought to change the description of this bug to "[patch] Improved methods to set and get PBX variables" or something similar.


By: Mark Spencer (markster) 2004-06-21 00:46:15

Yes, it's definitely not a good idea for variable_set to work on builtins.

By: Mark Spencer (markster) 2004-06-26 17:14:27

How is it coming?

By: hwstar (hwstar) 2004-06-28 11:01:32


I've been busy helping with app_rpt.c (Someone we both mutually know) Back to this bug: The ast_variable_get function currently has a problem  where it is returning garbage strings when builtin variables are requested. I'll need about week to sort it out.


edited on: 06-28-04 10:47

By: hwstar (hwstar) 2004-07-03 15:57:58

Problem with ast_variable_get fixed.  New tar file uploaded with new patch and
a test bench app to test out the new functions. The app should only be used for
testing and not included when the patch is merged into CVS.

By: Olle Johansson (oej) 2004-07-16 15:54:54

Housekeeping wonders if testing is successful? Where are we with this patch? Does it still apply to CVS? Do we need more testers?

By: hwstar (hwstar) 2004-07-16 16:19:46

Your answers:

aa. It tested out just fine on my system with the testbench included.

bb. I'd like to see other confirm the test results on their systems.

cc. Yes, I just applied the patch against a fresh checkout from the
top-o-the-heap -- 7/16/04 14:00

dd. See bb.


edited on: 07-16-04 16:04

By: Clod Patry (junky) 2004-08-27 00:43:47

I've just found something pretty interesting.
we can overwrite our global variables in the extensions.conf.
let me give ya a small example:
exten => s,1,setVar(EPOCH=${EPOCH});
exten => s,2,DeadAgi(init_agi.agi);

and into ur AGI, ya can now do a "GET VARIABLE EPOCH".
cause i've just overwritten the ${EPOCH} glob.var to the ${EPOCH} chan.var.

Maybe that would give an idea to someone to change something into the pbx_builtin_getvar_helper function.

By: Clod Patry (junky) 2004-10-25 23:26:17

anyone working on this problem/patch?


By: hwstar (hwstar) 2004-10-26 12:27:15


Correct me if I'm wrong, but your observation doesn't have anything to do with the functions I wrote: ast_variable_set(), ast_variable_get() and ast_variable_get_alloc(). The issue you are having is with the setVar() CLI command.

I'd like to suggest that we eiter a) break your issue out as a separate bug because if you follow the thread, the original intent has changed, or b) break out the additional functions as a separate bug to avoid confusion.


By: Brian West (bkw918) 2004-11-08 09:32:11.000-0600


EPOCH is already a var.


By: Mark Spencer (markster) 2004-11-16 23:53:07.000-0600

Added ImportVar app, should do it.