Summary:	ASTERISK-03742: [patch] Split huge language-specific stuff from say.c into different loadable modules
Reporter:	Paul Cadach (pcadach)	Labels:
Date Opened:	2005-03-22 15:14:54.000-0600	Date Closed:	2011-06-07 14:04:41
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/Internationalization
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) asterisk-lang2.diff
Description:	This patch simplifies addition of language-depended stuff into Asterisk, because each language located in its own source file. Architecture was discussed some time ago at IRC. ****** ADDITIONAL INFORMATION ****** Disclaimer is on file.
Comments:	By: Paul Cadach (pcadach) 2005-03-22 15:16:39.000-0600 At least all is compilable and runnable. I don't have test platform currently so can't check its functionality (but should work IMHO). By: Mark Spencer (markster) 2005-03-22 16:46:42.000-0600 Have you looked at the ASTOBJ stuff?  It might be helpful for managing the loading / unloading / reloading.  I'm a little bothered by the static set of function pointers.  I recommend instead the following: struct ast_lang {    const char *name;    int funccount;    void ((*funcs)(void))[AST_MAX_FUNCS]; }; Then we can statically define the functionc callbacks, e.g.: #define LANG_FUNC_SAY_NUMBER_FULL  0 #define LANG_FUNC_SAY_ENUMERATION_FULL 1 etc... When a language module registers itself, it can say what the max # of functions it has is, so that as we add more functions for a language module to supply, we don't ever overstep the bounds of the callbacks, e.g.: if (function_needed > lang->funccount) return NULL; else return lang->funcs[function_needed]; We can do some macro foo to make this easier for language to use. By: Kevin P. Fleming (kpfleming) 2005-03-22 17:06:39.000-0600 That sounds very fragile to me... What would be the value, other than allowing the use of binary-only language modules that don't support the full 'current' set of functions? By: Brian West (bkw918) 2005-03-22 18:24:28.000-0600 I like this.. I think citats and myself were in on this idea too.  I'll have to download this patch to see how it works... but having the stuff like this keeps people from hacking away at say.c every few days. /b By: Paul Cadach (pcadach) 2005-03-22 21:32:26.000-0600 Having array of function pointers instead of structure with set of function pointers would have to: 1) cast array item to specified function each time before call; 2) have possible problems to track function definitions (arguments, return type, etc.) compatible every place where it's used. Also, ast_lang_register()/ast_lang_unregister() currently have (ugly) mechanism for fix-up module's language structure to not have NULL functions (replacing undefined/NULL function references with ones from "default" language module, currently lang_en - an answer to kpfleming's question). "Ugly" is because fixups performed through defined fields only, and could be easily fixed by having "static"/"fixed" fields around a list of function pointers. By: Mark Spencer (markster) 2005-03-23 23:18:23.000-0600 The trouble is that the size of the registered block can change :-/ By: Mark Spencer (markster) 2005-03-23 23:19:22.000-0600 Wait, here's an idea..  there can be a sizeof(param) as the first element of the thing so that if the size is wrong we can at least just refuse to load the module.  What do you think? By: Kevin P. Fleming (kpfleming) 2005-03-23 23:59:49.000-0600 Right, but the size of the registered block can only change if someone is using a language module compiled against a different set of headers than the Asterisk core was compiled with. We don't make any accomodations for that for the channel_tech structure, or the musiconhold structure, or any other 'registration' structure. Throughout Asterisk it's assumed that a module that is going to register itself is using the same structure definition as the core it is registering with. By: Paul Cadach (pcadach) 2005-03-24 02:16:22.000-0600 There is another variance on Mark's opinion: 1) Instead of having sizeof(struct) within a struct better is to have call to ast_lang_register(&struct, sizeof(struct)) and ast_lang_unregister(&struct, sizeof(struct)); 2) Just define LANG_INTERFACE_VERSION at language.h and update it every time structure is changed, then update ast_lang_register()/ast_lang_unregister() to have second argument of LANG_INTERFACE_VERSION (which will be independed of size of structure and reflect changes in function prototypes too, for example). Then Asterisk's core could refuse to load language module if its version is different from one defined at core's build. 2Kevin: Because set of language modules could be changed there is possible to have newer modules in /var/lib/asterisk/modules/ when downgrading to old version with fewer languages, so check should be performed to make Asterisk be safe on such situations. By: Paul Cadach (pcadach) 2005-03-24 08:49:51.000-0600 Also, this is possible to use modutil's genksyms application to generate a hash depended on structure definition as version information. When structure definition is changed hash is changed too - that's good. Just add definitions: int ast_lang_version(struct ast_lang *); EXPORT_SYMBOL(ast_lang_version); then do next: gcc -E language.h | genksyms -k 2.1.20 | grep "^#define __ver" | sed -e 's/^\(#define[ ^I]*[A-Za-z0-9_]*[ ^I]*\)\([0-9a-fA-F]*\)$/\10x\2/' >language_ver.h and you will have language_ver.h file with unique hash value of struct ast_lang (and definition of ast_lang_version function, of course). But usage of genksyms is limited to Linux platform only... :( By: Kevin P. Fleming (kpfleming) 2005-03-24 09:01:36.000-0600 I think that adding a version field to the structure is definitely the right way to go, if we are going to start protecting the core from modules being loaded with older/newer structure definitions than expected. By: Paul Cadach (pcadach) 2005-03-24 23:18:58.000-0600 But this should be done globally IMHO, not for language modules only, because versioning/compatibility problem exists for all loadable modules due to dependences on internal structure definitions (like ast_channel, etc.). By: Mark Spencer (markster) 2005-03-26 01:26:49.000-0600 Generally speaking, I would say the language structure lends itself to much more volatility than the others, that's the only reason it catches my attention.  I don't think it's a show stopper, but the sizeof is an interesting way to solve this :) By: Mark Spencer (markster) 2005-03-30 00:57:06.000-0600 Lets get this updated for latest CVS, solicit comments from anyone who thinks this should be done differently, and go with it. By: Paul Cadach (pcadach) 2005-04-04 01:15:19 Reminder sent to oej Still waiting your opinion about design consideration before updating it for current CVS-HEAD. By: Olle Johansson (oej) 2005-04-04 03:21:11 Good work! This is according to our brainstorm a long time ago. From a brief look * We have to separate language, country and syntax * Modules register a SYNTAX * Several languages may use the same SYNTAX * A language code is not the same as a country code * Start with gettext's code list and syntax     ll_cc       ll = language code       cc = Country code  http://www.gnu.org/software/gettext/manual/html_mono/gettext.html * Do we also need language modules? For future localization of prompts, cli, help text etc? By: Paul Cadach (pcadach) 2005-04-07 15:52:47 Patch is updated for current CVS-HEAD. By: Mark Spencer (markster) 2005-05-23 20:25:12 Where does this one stand? By: Paul Cadach (pcadach) 2005-05-23 20:29:25 markster: The same place. Was ready for CVS-HEAD about 1,5 monthes ago. Probably is outdated now. I'll update it tomorrow. oej: gettext isn't requires any loadable module - translations is done "as is" by replacing format/text strings dynamically. By: Michael Jerris (mikej) 2005-05-25 12:30:23 Updated patch please? By: Paul Cadach (pcadach) 2005-05-26 01:25:03 Sorry, I'm ill a little. Patch will be available at the end of this week. By: Clod Patry (junky) 2005-06-23 23:11:43 PCadach: are you feeling better now? i hope everything's fine for you. You had time to update the patch now? By: Michael Jerris (mikej) 2005-07-20 19:34:51 bug suspended due to lack of response.  feel free to re-open when you have updates.