Re: [patch 1/1] net: convert %p usage to %pK

[Joe Perches <joe@perches.com>]

On Tue, 2011-05-24 at 09:04 +0200, Eric Dumazet wrote:
> Le mardi 24 mai 2011 à 08:57 +0200, Ingo Molnar a écrit :
> > * Joe Perches <joe@perches.com> wrote:
> > > Maybe for clarity it'd be better to use a switch/case
> > > or something like:
> Here we are, thanks.

Hey Eric.  Just more trivia:

> [PATCH v2] inet_diag: hide socket pointers
> Provide a mayber_hide_ptr() helper and use it in inet_diag to not

typo maybe_hide_ptr

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> @@ -798,6 +798,26 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
>  }
>  
>  int kptr_restrict __read_mostly;
> +/**
> + * maybe_hide_ptr - Eventually nullify a kernel pointer given to user

Not a great description.
Maybe something like:
 * maybe_hide_ptr - Set user values of kernel pointers to null
 *                  when appropriate
[]
> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> index 6ffe94c..b5646a3 100644
> --- a/net/ipv4/inet_diag.c
> +++ b/net/ipv4/inet_diag.c
> @@ -84,6 +84,7 @@ static int inet_csk_diag_fill(struct sock *sk,
>  	struct inet_diag_meminfo  *minfo = NULL;
>  	unsigned char	 *b = skb_tail_pointer(skb);
>  	const struct inet_diag_handler *handler;
> +	u64 ptr;
>  
>  	handler = inet_diag_table[unlh->nlmsg_type];
>  	BUG_ON(handler == NULL);
> @@ -114,8 +115,9 @@ static int inet_csk_diag_fill(struct sock *sk,
>  	r->idiag_retrans = 0;
>  
>  	r->id.idiag_if = sk->sk_bound_dev_if;
> -	r->id.idiag_cookie[0] = (u32)(unsigned long)sk;
> -	r->id.idiag_cookie[1] = (u32)(((unsigned long)sk >> 31) >> 1);
> +	ptr = (u64)maybe_hide_ptr(sk);
> +	r->id.idiag_cookie[0] = (u32)ptr;
> +	r->id.idiag_cookie[1] = (u32)(ptr >> 32);

I think it's be better without the casts
using the standard kernel.h macros.

	void *ptr;

	ptr = maybe_hide_ptr(sk);
	r->id.idiag_cookie[0] = lower_32_bits(ptr);
	r->id.idiag_cookie[1] = upper_32_bits(ptr);