Messages in this thread | | | Date | Fri, 02 Mar 2007 16:11:01 +0000 | From | Paulo Marques <> | Subject | Re: [PATCH 2.6.21-rc1] Extend print_symbol capability |
| |
Robert Peterson wrote: > [...] > #define KSYM_NAME_LEN 127 > +#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \ > + 2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1) > > #ifdef CONFIG_KALLSYMS > /* Lookup the address for a symbol. Returns 0 if not found. */ > @@ -22,6 +24,9 @@ const char *kallsyms_lookup(unsigned long addr, > unsigned long *offset, > char **modname, char *namebuf); > > +/* Look up a kernel symbol and return it in a text buffer. */ > +extern void lookup_symbol(unsigned long addr, char *buffer);
I don't like this name much :(
We already have kallsyms_lookup and kallsyms_lookup_name. The name of this function should imply that it will print the formatted result into the buffer, not just lookup a symbol.
Maybe "__sprint_symbol", and change the interface to "__sprint_symbol(char *buffer, unsigned long addr)"?
> + > /* Replace "%s" in format with address, if found */ > extern void __print_symbol(const char *fmt, unsigned long address); > > @@ -47,6 +52,11 @@ static inline const char *kallsyms_lookup(unsigned > long addr, > return NULL; > } > > +static inline void lookup_symbol(unsigned long addr, char *buffer) > +{ > + return NULL; > +}
Returning NULL in a function returning "void" doesn't seem right :P
Maybe it should be something like this instead: { *buffer = '\0'; }
> [...]
Anyway, the change looks useful, so thanks for the patch :)
-- Paulo Marques - www.grupopie.com
"Very funny Scotty. Now beam up my clothes." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |