On 5/10/09, Andreas Robinson <andr345@xxxxxxxxx> wrote: > Hi guys, > > following Robby's suggestion, I have moved to github. > > Please have a look at > http://github.com/andr345/module-init-tools/commits/master/ > > (or git://github.com/andr345/module-init-tools.git) > > This is the second set of patches in the ELF-rewrite, organized for easy > review. 1) I don't think load_strings() needs to be exported in struct module_ops. It's easy enough to open-code and it assumes you need the results in a stringtbl. It looks like it's just an implementation detail of load_symbols(). Just like deref_sym() is an implementation detail of fetch_tables(). 2) "stringtbl" looks a little out of place here. And I'm uneasy about the way it's used for constant strings (weak ? "W" : "U") in load_dep_syms(), as well as for malloc()'d strings. I think a linked list with a field for the "weak" value would fit in better. Allocate the string inline, like "struct symbol", and readers don't have to worry about the string allocation as a separate issue. You could call it "struct elf_symbol" to distinguish it. 3) I tried running depmod under valgrind (you have to restrict the number of modules, i.e. just copy a few modules to a staging directory and use the -b option). Previously, depmod was valgrind-clean. ==12690== Conditional jump or move depends on uninitialised value(s) ==12690== at 0x804A095: output_deps_bin (depmod.c:473) ==12690== by 0x804CD85: main (depmod.c:1364) ==12690== Uninitialised value was created by a stack allocation ==12690== at 0x80517D3: add_value (index.c:86) ==12690== ==12690== Conditional jump or move depends on uninitialised value(s) ==12690== at 0x804B066: output_symbols_bin (depmod.c:783) ==12690== by 0x804CD85: main (depmod.c:1364) ==12690== Uninitialised value was created by a stack allocation ==12690== at 0x80517D3: add_value (index.c:86) ==12690== ==12690== ERROR SUMMARY: 4 errors from 2 contexts (suppressed: 11 from 1) ==12690== malloc/free: in use at exit: 12,849 bytes in 138 blocks. ==12690== malloc/free: 1,191 allocs, 1,053 frees, 321,180 bytes allocated. Did you try "tests/runtests --valgrind" ? Hopefully that should pick it up as well, but it's a bit slow :-). Regards Alan -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
