On Wed Jun 16 2010 at 15:36:30 -0700, Paul Goyette wrote: > The attached diffs add one more routine, module_init3() which gets > called from init_main() right after module_class_init(MODULE_CLASS_ANY). > module_init3() walks the list of builtin modules that have not already > been init'd and marks them disabled. > > Tested briefly on my home systems and appears to work. > > Any objections to committing this?
I'd still hook it to the end of module_class_init(MODULE_CLASS_ANY) instead of adding more randomly numbered module_init<n>() calls. The other benefit from doing so is that you get it done atomically, which is always worthwhile, and doubly so when it's a low hanging fruit like here. > @@ -416,6 +434,7 @@ module_init_class(modclass_t class) > * init. > */ > if (module_do_builtin(mi->mi_name, NULL) != 0) { > + mod->mod_disabled = true; > TAILQ_REMOVE(&module_builtins, mod, mod_chain); > TAILQ_INSERT_TAIL(&bi_fail, mod, mod_chain); > } Why do you mark it as disabled? Doesn't this conflict with the "it might succeed in a later module_init_class()" idea you presented earlier? module_disabled = true/false in multiple places looks a little error-prone. Now that struct module is growing more and more members, maybe we can just have an object allocator which initializes the value and afterwards the only acceptable mutation for module_disabled is setting it to true (might make sense to rename the variable to something like module_virgin and flip the polarity, though).