Extend print_symbol capability
PATCH 1/1 Extend print_symbol capability
Today's print_symbol function dumps a kernel symbol with printk. This patch extends the functionality of kallsyms.c so that the symbol lookup function may be used without the printk. This is useful for modules that want to dump symbols elsewhere, for example, to debugfs. I intend to use the new function call in the GFS2 file system (which will be a separate patch).
---
include/linux/kallsyms.h | 10 ++++++++++ kernel/kallsyms.c | 21 ++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-)
include/linux/kallsyms.h +0 -0
@@ -7,6 +7,8 @@
#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); +
/* 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;
+}
+
/* Stupid that this does nothing, but I didn't create this mess. */ #define __print_symbol(fmt, addr)
#endif /*CONFIG_KALLSYMS*/
kernel/kallsyms.c +0 -0
@@ -267,20 +267,17 @@ const char *kallsyms_lookup(unsigned long addr,return NULL ;
}
-/* Replace "%s" in format with address, or returns -errno. */ -void __print_symbol(const char *fmt, unsigned long address) +/* Look up a kernel symbol and return it in a text buffer. */ +void lookup_symbol(unsigned long addr, char *buffer)
{
char *modname;
const char *name;
unsigned long offset, size;
char namebuf[KSYM_NAME_LEN+1];- char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN +
- 2*(BITS_PER_LONG*3/10) + MODULE_NAME_LEN + 1];
-
- name = kallsyms_lookup(address, &size, &offset, &modname, namebuf);
+ name = kallsyms_lookup(addr, &size, &offset, &modname, namebuf);
if (!name)
- sprintf(buffer, "0x%lx", address);
+ sprintf(buffer, "0x%lx", addr);
else {
if (modname)
sprintf(buffer, "%s+%#lx/%#lx [%s]", name, offset,
@@ -288,6 +285,15 @@ void __print_symbol(const char *fmt, unsigned long address)
else
sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
}+}
+
+/* Replace "%s" in format with address, or returns -errno. */ +void __print_symbol(const char *fmt, unsigned long address) +{
+ char buffer[KSYM_SYMBOL_LEN];
+
+ lookup_symbol(address, buffer);
+
printk(fmt, buffer); }
@@ -452,3 +458,4 @@ static int __init kallsyms_init(void) __initcall(kallsyms_init);
EXPORT_SYMBOL(__print_symbol);
+EXPORT_SYMBOL(lookup_symbol);
Review const char *kallsyms_lookup(unsigned long addr, · line 28
Quoted 13 lines · Robert Peterson
[...]
#defineKSYM_NAME_LEN127(quoted diff) +2
-0+#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. */(quoted diff) +2
-0@@ -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)"?
Review static inline const char *kallsyms_lookup(unsigned · line 59
Quoted 13 lines
(quoted diff) +5
-0+ /* 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';
}Quoted 1 line
[...]
Anyway, the change looks useful, so thanks for the patch :)
Signature · 3 lines
Paulo Marques - www.grupopie.com "Very funny Scotty. Now beam up my clothes."
Quoted 5 lines · Paulo Marques
I don't like this name much :(
We already have
kallsyms_lookupandkallsyms_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)"?
I'm not sure I like the leading __. In the print_symbol case, I think the function was given a leading __ so that code referencing print_symbol would use the macro which formulates the call into __print_symbol. I don't mind sprint_symbol though. Since Andrew Morton included the patch, I'll defer to his judgment.
Quoted 5 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+static inline void lookup_symbol(unsigned long addr, char *buffer) +{ + return NULL; +}Returning NULL in a function returning "void" doesn't seem right :P
You're right. This should just be a simple "return;". My bad. Good catch. Since Andrew Morton has already included this patch, I'll let him make this change if he sees fit.
Regards,
Bob Peterson
Red Hat Cluster Suite