Re: [PATCH 2/2] module: fix bne2 "gave up waiting for init of modulelibcrc32c"

[Linus Torvalds]

On Thu, 3 Jun 2010, Rusty Russell wrote:
> 
> However, you're right that it has potential.  I'll rename module_info to
> load_info if you don't mind tho: contains more semantic punch IMHO.

Umm. One problem is that you will almost certainly eventually want to 
expose that to the architecture "fixup" routines (ie things like 
module_frob_arch_sections(), arch_mod_section_prepend()), and at that 
point "load_info" is a horribly bad structure name, since it would show 
up in <linux/module.h> and thus be exported all over.

At least call it "struct module_load_info". But yes, I do agree that the 
"load" part is important.

> On top of this, I'm right now closing on another ideal of mine: encapsulate
> all the "before we move module" into one function.  That before vs. after
> always made me nervous...

Yeah, that should be trivial, and I agree that it would be good to not 
have "mod" mean two things in the same function. Especially with all the 
"goto failure-case", and some of the failure cases using "mod", it is a 
bit scary for it to point into the (before movement) 'hdr+len' structure, 
and then (after movement) into the relocated module allocations.

I looked at that particularly when doing that whole

	mod = setup_module_info(&info);
	if (IS_ERR(mod)) {
		err = PTR_ERR(mod);
		goto free_hdr;
	}

thing, because that made "mod" have _three_ totally different values 
(error, before, after) when jumping out to the failure paths. 

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/