Attention is currently required from: Kevin Harwell.
N A has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/17792 )
Change subject: pbx_variables: Add variable registration and validation
......................................................................
Patch Set 1:
(2 comments)
File main/pbx_variables.c:
https://gerrit.asterisk.org/c/asterisk/+/17792/comment/9313359e_c3bdadbc
PS1, Line 116: static struct varshead systemvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
: static struct varshead specialchanvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
: static struct varshead modvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
: static struct varshead modeditvars = AST_LIST_HEAD_NOLOCK_INIT_VALUE;
> Unless there are 10's of thousands or more you probably don't have to worry much about performance o […]
I'll look into the vector approach. Yes, things would really only be added whenever modules load, so more or less when Asterisk starts. I think we should optimize for fast search since this could be used a LOT during runtime (read only), but if adding things is a little bit slower, that wouldn't be a big deal, as it's more or less read-only after modules are loaded.
https://gerrit.asterisk.org/c/asterisk/+/17792/comment/3fed393a_ce7d9847
PS1, Line 155: int ast_varname_reserved(const char *name)
: {
: char *tmp = (char*) name;
: /* Asterisk broadly reserves all uppercase + underscore var names for use by Asterisk */
: while (*tmp) {
: if (tmp[0] != '_' && !isupper(tmp[0])) {
: return 0;
: }
: tmp++;
: }
: return 1;
: }
> I misread the code a bit here. But now I see it appears to return 0 (i.e. […]
Hmm... it seemed this was a longstanding unofficial "rule" but maybe it's more legendary myth at this point than an established policy.
For instance, from p. 97 of Asterisk: The Definitive Guide 5th ed.: "Note that variable names are case-sensitive. A variable named SOME DIGITS is different from a variable named SomeDigits. You should also be aware that any variables set by Asterisk will be uppercase. Some variables, such as CHANNEL and EXTEN, are reserved by Asterisk".
So Asterisk is reserved to using only variable names consisting solely of uppercase letters and underscores.
When I started with Asterisk, I used a few all uppercase var names for globals and was quickly scolded by someone who'd been using it for 20+ years now:
"Variable names. Any Variable name which does not include at least ONE lower-case letter is reserved for use by the Asterisk developers. You should never use variable names that are all upper case, because you might run into something that gets added in the future or is undocumented."
So somehow or another, this seems to have become entrenched amongst the user base and is at least implicitly observed by developers.
A good example of the latter point is some of the internal variables that aren't documented anywhere in individual modules, such as in app_while how it stores information in variables for its own use. These variables wouldn't get registered since they're not user-facing, and a user could happen to use the same name and wonder why things are getting messed up. But this is probably an edge case.
All this said, and that last point aside, the upside of variable registration is if there is truly a conflict, we can find it and warn about it, so maybe it's not necessary to warn about a possible conflict at large, unless we've established that a conflict in fact exists. So I can probably take out this part either way.
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/17792
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: Iad3b8d9833c7d9debe04aca59260d7316c3ad28c
Gerrit-Change-Number: 17792
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Attention: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Fri, 14 Jan 2022 01:11:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: N A <mail at interlinked.x10host.com>
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220113/0575032f/attachment.html>