On Wed, 9 Jan 2008 14:20:18 +1100 Rusty Russell <[EMAIL PROTECTED]> wrote:
> On Wednesday 09 January 2008 11:21:59 Andrew Morton wrote: > > The string handling in here has become a bit scruffy. > > Yes, that patch also evokes a const warning. Fixed below. No patch was included. > I assume you've > queued these because you're thinking of applying them before 2.6.24? I'd say > only modules-de-mutex-more-symbol-lookup-paths-in-the-module-code.patch > warrants that (the other is unlikely and not a regression). Actually I was thinking 2.6.25 on both. <looks> Kyle McMartin reports sysrq_timer_list_show() can hit the module mutex; these paths don't need to though, since we long ago changed all the module list manipulation to occur via stop_machine(). Disabling preemption is enough. Ah. sysrq_timer_list_show() is called from interrupt. <fixes changelog, thwaps its author> OK, 2.6.24 seems reasonable. > > afacit the `namebuf[KSYM_NAME_LEN - 1] = 0;' would be unneeded if we were > > to use strlcpy() and I suspect the `namebuf[0] = 0;' isn't needed either. > > > > And the use of strlcpy() means we don't need to subtract 1 from > > KSYM_NAME_LEN and we don't need to fret about weird strncpy semantics when > > the input string is too large. > > > > > > And the fact that incoming arg `namebuf' MUST point at a > > KSYM_NAME_LEN-sized buffer could be better communicated by using a > > dedicated struct for this, or by giving the arg a type of `char > > namebuf[KSYM_NAME_LEN]'. Or by adding a comment. Or by just ignoring > > me and doing something more useful. > > Or better, rework all the name lookup interfaces, rather than having: > > struct module *module_text_address(unsigned long addr); > struct module *__module_text_address(unsigned long addr); > int is_module_address(unsigned long addr); > int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type, > char *name, char *module_name, int *exported); > char *module_address_lookup(unsigned long addr, > unsigned long *symbolsize, > unsigned long *offset, > char **modname, > char *namebuf); > int lookup_module_symbol_name(unsigned long addr, char *symname); > int lookup_module_symbol_attrs(unsigned long addr, unsigned long *size, > unsigned long *offset, char *modname, char > *name); > unsigned long module_kallsyms_lookup_name(const char *name); > > unsigned long kallsyms_lookup_name(const char *name); > extern int kallsyms_lookup_size_offset(unsigned long addr, > unsigned long *symbolsize, > unsigned long *offset); > const char *kallsyms_lookup(unsigned long addr, > unsigned long *symbolsize, > unsigned long *offset, > char **modname, char *namebuf); > extern int sprint_symbol(char *buffer, unsigned long address); > extern void __print_symbol(const char *fmt, unsigned long address); > int lookup_symbol_name(unsigned long addr, char *symname); > int lookup_symbol_attrs(unsigned long addr, unsigned long *size, > unsigned long *offset, char *modname, char *name); Yes, it could all do with a revisit. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
