[PATCH 2/3] move loading of modules out of the AddModule call back

[Andrei Kholodnyi]

On Thu, Nov 18, 2010 at 12:40 PM, Trevor Saunders
<trev.saunders at gmail.com> wrote:
> We know use the AddModule call back to create a list of modules we wnat
> to try and load. ?Then after parsing the config file we try to load all
> the modules.
> ---
> ?src/server/configuration.c | ? 32 ++++++++++++--------------------
> ?src/server/configuration.h | ? ?6 ++++++
> ?src/server/module.c ? ? ? ?| ? 15 ++++-----------
> ?src/server/speechd.c ? ? ? | ? 21 ++++++++++++++++++++-
> ?4 files changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/src/server/configuration.c b/src/server/configuration.c
> index 6306015..6b260c0 100644
> --- a/src/server/configuration.c
> +++ b/src/server/configuration.c
> @@ -275,35 +275,27 @@ DOTCONF_CB(cb_CustomLogFile)
>
> ?DOTCONF_CB(cb_AddModule)
> ?{
> + ? ? ? OutputModule *module = g_malloc(sizeof(OutputModule));
> ? ? char *module_name;
> ? ? char *module_prgname;
> ? ? char *module_cfgfile;
> ? ? char *module_dbgfile;
>
> - ? ?OutputModule *cur_mod;
>
> - ? ?if (cmd->data.list[0] != NULL) module_name = g_strdup(cmd->data.list[0]);
> - ? ?else FATAL("No output module name specified in configuration under 
> AddModule");
> -
> - ? ?module_prgname = cmd->data.list[1];
> - ? ?module_cfgfile = cmd->data.list[2];
> -
> - ? ?module_dbgfile = g_strdup_printf("%s/%s.log", SpeechdOptions.log_dir,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?module_name);
> -
> - ? ?cur_mod = load_output_module(module_name, module_prgname, module_cfgfile,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?module_dbgfile);
> - ? ?if (cur_mod == NULL){
> - ? ? ? ?MSG(3, "Couldn't load specified output module");
> - ? ? ? ?return NULL;
> + ? ?if (cmd->data.list[0] != NULL){
> + ? ? ? ?module->name = g_strdup(cmd->data.list[0]);
> + ? ?}else{
> + ? ? ? ?MSG(3, "No output module name specified in configuration under 
> AddModule");
> + ? ? ? return NULL;
> ? ? }
>
> - ? ?assert(cur_mod->name != NULL);
> - ? ?output_modules = g_list_append(output_modules, cur_mod);
> - ? ?MSG(5,"Module name=%s being inserted into modules list", cur_mod->name);
> + ? ?module->filename = g_strdup_printf("%s/%s", MODULEBINDIR, 
> cmd->data.list[1]);
> + ? ?module->configfilename = g_strdup_printf("%s/modules/%s", 
> SpeechdOptions.conf_dir, cmd->data.list[2]);
> + ? ?module->debugfilename = g_strdup_printf("%s/%s.log", 
> SpeechdOptions.log_dir,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?module->name);
>
> - ? ?g_free(module_dbgfile);
> - ? ?g_free(module_name);
> + ? ?loadable_modules = g_list_append(loadable_modules, module);
> + ? ?MSG(5,"Module name=%s being inserted into modules list", module->name);
>
> ? ? return NULL;
> ?}

I do not see any reason to make changes here. it should continue to
load modules as before

> +GList *detect_modules(void);

function declared in this patch, but not implemented


> +GList *loadable_modules = NULL;

this variable is not needed

> + ? ?while(NULL != loadable_modules){
> + ? ? ? ? ? OutputModule *new_module = NULL, *to_load = 
> loadable_modules->data;
> +
> + ? ? ? ? ? new_module = load_output_module(to_load->name, to_load->filename, 
> to_load->configfilename, to_load->debugfilename);
> +
> + ? ? ? ? ? if(new_module != NULL)
> + ? ? ? ? ? ? ? ?output_modules = g_list_append(output_modules, new_module);
> +
> + ? ? ? ? ? g_free(to_load->name);
> + ? ? ? ? ? g_free(to_load->filename);
> + ? ? ? ? ? g_free(to_load->configfilename);
> + ? ? ? ? ? g_free(to_load->debugfilename);
> + ? ? ? ? ? g_free(to_load);
> + ? ? ? ? ? loadable_modules = g_list_delete_link(loadable_modules, 
> loadable_modules);
> + ? ?}

here it should be something like:

if (output_modules == NULL)
   while (read dir entry) {
       module_name = g_strdup(entry->d_name+3);
       module_prgname = g_strdup_printf("%s/%s", MODULEBINDIR, entry->d_name);
       module_cfgfile = g_strdup_printf("%s/modules/%s",
SpeechdOptions.conf_dir, module->name);
       module_dbgfile = g_strdup_printf("%s/%s.log",
SpeechdOptions.log_dir, module->name);
       cur_mod = load_output_module(module_name, module_prgname,
module_cfgfile, module_dbgfile);
       if(cur_mod != NULL)
            output_modules = g_list_append(output_modules, cur_mod);
  }

patches 2 and 3 can be merged in one.
If you agree, I could do these changes and send them for review.