Re: [PATCH 2.6.21-rc1] Extend print_symbol capability TRY #3

[Paulo Marques <pmarques@grupopie.com>]

Robert Peterson wrote:
>[...]
> @@ -47,6 +52,11 @@ static inline const char *kallsyms_lookup(unsigned 
> long addr,
>     return NULL;
> }
> 
> +static inline void sprint_symbol(char *buffer, unsigned long addr)
> +{
> +    return;
> +}

I'm really sorry for not replying sooner (I've been really busy), but 
this function still doesn't seem right.

If someone does something like:

> void my_function(unsigned long addr)
> {
> 	char buffer[KSYM_SYMBOL_LEN];
> 
> 	sprint_symbol(buffer, addr);
> 	...
> 	// use buffer to print somewhere
> 	...
> }

which seems like a perfectly natural thing to do, it might just oops the 
kernel if CONFIG_KALLSYMS is not set, because the buffer will be left 
uninitialized.

That is why I suggested to change it to something like "*buffer = '\0'" 
instead.

The really nice solution IMHO, would be to remove the print_symbol and 
sprint_symbol functions from the the "#ifdef CONFIG_KALLSYMS" and just 
let them be available even in a not CONFIG_KALLSYMS kernel.

Since kallsyms_lookup is already #ifdef'ed to something sane, 
sprint_symbol will just print out the symbol address in that case, but 
it is better than not printing anything at all.

-- 
Paulo Marques - www.grupopie.com

"Very funny Scotty. Now beam up my clothes."