[Asterisk-Dev] loader issues.

[Kevin P. Fleming]

Luigi Rizzo wrote:
> I think there are two issues in the current loading scheme.
> First is a namespace pollution - by forcing each module 
> have global symbols with 'common' names such as key, usecount,
> reload, description we cannot easily run with, say, -Wshadow
> or other stricter compiler checks.
> Putting all callbacks in a single descriptor would ease life
> to the compiler and the developer by reducing the namespace
> pollution.

I have some plans to do this sort of thing, but not until after the 1.2 
release branch is made. In fact, I want to add even more stuff:

- when a module is loaded, it should be handed a 'handle' it can use for 
other functions
- when applications, functions, CLI commands, etc. are registered, they 
would be registered under the module's handle, and automatically 
unregistered when the module is unloaded
- the list of apps/functions/CLI commands/etc. to register would be 
statically defined, and automatically registered if the module's 
load_module() function returns a success value (although individual 
entries could still be registered/unregistered if desired)

> In fact, both things seem to me dangerous for two reasons:
> 1) by using RTLD_LAZY, you defer symbol resolution at a later time,
>    which is probably the opposite of what you want -- surely
>    you want to know of unresolved symbols at the time you load a
>    module and not much later perhaps in the middle of some unrelated
>    action;

Except that won't work for modules with references to each other, or if 
modules are loaded in the wrong order (although that's less of a concern).

> 2) RTLD_GLOBAL opens the door to unwanted bugs, e.g. a module defining
>    global symbols by mistake, and making them available to others in
>    unwanted ways.

This is a valid concern, but would require a massive restructuring of 
module handling (as you've proposed).

> void *sym_lookup(const char *sym)
> {
>         struct module *m;
>         struct exported_symbols *e;
> 
>         /* lock modlock */
>         for (m = module_list; m ; m = m->next)
>                 for (e = m->s; e && e->name; e++)
>                         if (!strcmp(e->name, sym) {
>                                 /* update m->usecount */
>                                 /* unlock modlock */
>                                 return e->value;
>                         }
>         /* not found */
>         /* unlock modlock */
>         return NULL;
> }

I like this idea, but primarily because of the usecount handling.