linux-kernel - Re: [PATCH 2.6.21-rc1] Extend print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <45E84C95.3060604@grupopie.com>
Date:	Fri, 02 Mar 2007 16:11:01 +0000
From:	Paulo Marques <pmarques@...popie.com>
To:	Robert Peterson <rpeterso@...hat.com>
CC:	linux-kernel@...r.kernel.org
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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/