[Home]

Summary:ASTERISK-02965: [PATCH] allow contexts to be repeated in multiple files, so they add together
Reporter:Kevin P. Fleming (kpfleming)Labels:
Date Opened:2004-12-11 00:05:58.000-0600Date Closed:2005-02-06 21:44:49.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Core/Configuration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) config_context_addition_rev8.diff.txt
Description:The patch actually includes quite a bit of cleanup and reorg of config.c... I hadn't intended to do that, but it happened as a side effect.

In any case, the purpose of this patch is to allow the following:

(extensions.conf)
[dundi-inbound]
exten => 1,1,foo
exten => 2,1,foo
#include customer/extensions.conf

(customer/extensions.conf)
[+dundi-inbound]
exten => 3,1,foo

The net result is that the contexts are combined together, as if they had occurred together in a single sequence. This works no matter how many times the same context occurs in as many files as are needed.

There is one side effect of this patch: repetition of the same context name _without_ adding a + symbol now generates an error, instead of being ignored as it was previously.

****** ADDITIONAL INFORMATION ******

Disclaimer on file.
Comments:By: Mark Spencer (markster) 2004-12-11 00:15:15.000-0600

Repitition did not ignore, it created two entries.  For example:

[foo]
type=user
.
.

[foo]
type=peer
.
.

is totally fine and does the right thing.  might need to revisit that.  why not just include after the [] rather than add another?
By: Kevin P. Fleming (kpfleming) 2004-12-11 00:22:08.000-0600

Ahh, you are right, repetition was OK. I'll revise that.

I'm not sure I follow your other question... the reason I think is useful is primarily in combination with the wildcard include patch. This way, it's possible to "build up" a context in extensions.conf from an unknown number of other files, depending on what is included by the wildcard.

In my specific case, I'm using it to build up a context for TNs to accept calls via DUNDi, and each customer gets to control their list. My master extensions.conf file doesn't even have to know about which customers have decided to participate and which have not; all the customer extensions.conf files are included (via wildcard), and then some (or all) of them can add entries to the single DUNDi inbound context.
By: Kevin P. Fleming (kpfleming) 2004-12-11 00:24:55.000-0600

rev2 uploaded, no longer warns/errors on duplicate context names
By: Clod Patry (junky) 2004-12-16 22:38:13.000-0600

i was unable to apply to patch to my current version: CVS-HEAD-12/16/04-21:33:24

Are you sure it's still applicable?

I know anthm's patch modified a lot config.c to supports multi-lines comments (see 0002911).
By: Kevin P. Fleming (kpfleming) 2004-12-16 23:28:19.000-0600

I did my patch after the multi-line comments changes went in, but there have been more config.c changes since then... I'll post a new rev tomorrow.
By: Kevin P. Fleming (kpfleming) 2004-12-18 21:03:46.000-0600

rediffed against current CVS, and added example usage to extensions.conf.sample
By: Kevin P. Fleming (kpfleming) 2005-01-03 18:06:09.000-0600

Patch is now relative to the cleanup patch in ASTERISK-3203237.

edited on: 01-03-05 18:11
By: Anthony Minessale (anthm) 2005-01-05 17:48:23.000-0600

This would help external config engines too, if you wanted to add some stuff from an external db into an existing context although it would seem to be more elegant to not need a +

[mycon]
exten => 1,1,Hangup

#include dbextensions

dbextensions (wired to res_config) may want to add to mycon but doesnt know if it exists or not.
By: Kevin P. Fleming (kpfleming) 2005-01-05 23:30:08.000-0600

Yes, I had thought of this (combining text-based and db-based configs). Unfortuantely there has to be some sort of indicator that you intend for the contexts to combine together, because some modules do support multiple contexts with the same name (sip.conf, iax.conf, maybe others).
By: Anthony Minessale (anthm) 2005-01-06 09:21:41.000-0600

How bout a new preprocessor directive so you can flip meta variables
relevant to the configuration?

#combine_catrgories => true
By: Kevin P. Fleming (kpfleming) 2005-01-06 09:40:52.000-0600

Well, if we go that route, I'd take a cue from the C preprocessor (since we already use #include) and use #pragma...

#pragma duplicate-categories no

That way it can be extended without needing lots of additional directives... although there's probably a better word than "pragma" for non-programmers :-)
By: Anthony Minessale (anthm) 2005-01-06 09:57:46.000-0600

i'm happy with anything just the fundamental fact that you cold flip a bit to say "ok, combine cats if you encounter duplicates" is the part that matters to me I'm sure markster will be able to make that call on the semantics =D
By: Kevin P. Fleming (kpfleming) 2005-01-06 10:12:09.000-0600

In fact I'd prefer an implementation that worked that way, and I'd also prefer to have ast_load() be able to feed in the default value for that flag, so that chan_sip/chan_iax2/etc. could turn it off, but everywhere else it would default to "on".

Really, there are very few config files in Asterisk where duplicate context names actually make sense, so in my opinion it would be better to disable them except where they are needed.

How about this for an implementation:

- add to config.h a set of CONFIG_FLAG variables for future features (with first one being CONFIG_FLAG_DUPLICATE_CONTEXTS)

- modify ast_load to accept a "struct flags *" with the initial flags to be used for that parsing run (NULL means no initial flags turned on)

- add a new directive to the parser called "#parser" that can be used to turn flags on and off during the parsing run; the first available flag would be "duplicate-contexts"... usage would be "#parser duplicate-contexts yes|no|true|false"

- I don't know how the parsing flags could be used to enable combination of text- and db-based config, but I suspect it could be done.

I know Mark has said he wants to keep the config parser "simple", but this would be a worthwhile addition in my opinion, and easily extendible without complicating the parser once it is in place.

Tony, if you like this I'd be happy to code it up and get it posted for review.
By: Anthony Minessale (anthm) 2005-01-06 11:04:38.000-0600

how bout just
static int default_config_flags = CONFIG_FLAG_DUPLICATE_CONTEXTS;
at the top of config.c

Then ast_load can have its own local copy

/* start with defaults */
int config_flags = default_config_flags;

then you can use the flag macros that already exist

if(ast_test_flag(config_flags,CONFIG_FLAG_DUPLICATE_CONTEXTS)) {
....
}

ast_set_flag(config_flags,CONFIG_FLAG_DUPLICATE_CONTEXTS);

or .. This one despite it's unsual name is moreso ast_set_flag_if_true or
something along those lines so you could make a front end pretty easy to have

paraphrased code ...

#setflag duplicate_contexts 1

if (!strcasecmp(cur, "setflag")) {
 if((flagname = strchr(cur,' '))) {
   flagname++;
   if((flagval = strchr(flagname,' '))) {
     if(!strcmp(flagname,"duplicate_contexts"))      
        ast_set2_flag(config_flags, ast_true(flagval), CONFIG_FLAG_DUPLICATE_CONTEXTS);
     
   }
 }
}

1 thing, context is only a construct of extensions so you may want to call it category which is it's real name dunno I try not to get too emotional on semantics cos it changes a lot at this stage =D

drop by the clue channel if you want to discuss it.

edited on: 01-06-05 11:05
By: Kevin P. Fleming (kpfleming) 2005-01-08 19:27:59.000-0600

New version (rev6), thanks to anthm for pushing me in the right direction :-)

This is a new implementation, that does not require any prefix characters on context names to allow addition of contexts together. Instead, addition is allowed by default (just by using the same name more than once), unless the module doing the config load disallows it (which chan_sip and chan_iax2 now do).

This means that you can add contexts together even across multiple config sources (text, realtime, etc.) without requiring special context names.

In addition, the ability to duplicate context names can be controlled within the config file itself (for text-based configs) by using:

#parser duplicate-categories [on|off|yes|no|1|0]

Future parser control options can be added using the code already provided.

Patch is still relative to ASTERISK-3203237.

edited on: 01-08-05 19:29
By: Anthony Minessale (anthm) 2005-01-09 09:19:09.000-0600

I like this!  db configuration will benifit the most so you can have 1/2 static 1/2 dymaic entries to the same category much easier.

Maybe we should have an option somewhere extconfig.conf or asterisk.conf to set the default behaviour

multiplecats => combine|ignore

Then you can have perfect prepatch behaviour preserved to spare any unforseen damage it could cause and accomidate somebody for whatever reason doesnt want that behaviour (not me I think it rocks!)
By: Kevin P. Fleming (kpfleming) 2005-01-09 09:48:57.000-0600

New rev, fixing stupid thinko bug in ast_load_with_flags :-(
By: Kevin P. Fleming (kpfleming) 2005-01-15 20:09:02.000-0600

Rediffed against current CVS and patch in ASTERISK-3203237.
By: Mark Spencer (markster) 2005-01-16 00:51:15.000-0600

Again, I think the complexity here is getting out of hand.  What was wrong with the simple idea that you can put + in front of the name, either:

+[foo]
or
[+foo]
or even
[foo]+

whichever you feel better with (the last perhaps is my favorite because of the simplicity to implement/test)

This could be accomplished with no new "#foo" nonsense, no module specific behaviors, no flags or other superfluous nonsense.
By: Kevin P. Fleming (kpfleming) 2005-01-16 08:48:13.000-0600

You'll have to ask anthm and the others who commented about that... Personally I was happy with using [+foo] to indicate addition, and I can certainly go back to to that.

However, I can also see value in supporting directives to control the parser's behavior, and this would be a good candidate to introduce that with.
By: Kevin P. Fleming (kpfleming) 2005-01-16 13:20:28.000-0600

Functionality has been moved into ASTERISK-3063099.
By: Anthony Minessale (anthm) 2005-01-17 14:41:18.000-0600

The + thing really belongs being the default so you would need to generate an extra + all the time all over the place. when in reality, sip and iax are the only ones that need the non + way you should make them have to say [whatever]-
then.

The superfluous nonsense simply adds 1 meesly int to the config obj opening the door to future flag handling possiblitly (see all the time we've spent going back and adding flags when it was obvious it should have been implemented that way from the start?) the # nonsense simple extends an existing functionality to allow tuning of these flags. Let's take this white glove and go run it along the several kilobytes of pure crap that has been willingly committed over political pressures etc.  I have corrected so many needless strdups and uninitilized pointers i wish i had a dime for each one.
If we want standards, how bout no more 1 character varnames unless designated , ok f can be a frame but 'p' means EVERYTHING ??? I was invited to make a list of rules recently I think I will take you up on that.
By: Mark Spencer (markster) 2005-02-03 00:11:35.000-0600

This functionality is already merged in from the earlier inheritance patch (just use '+' in the list and it appends to the preexisting category).