[PATCH] net: clean up some sparse endianness warnings in ipv6.h

[PATCH] net: clean up some sparse endianness warnings in ipv6.h

[Jeff Layton]

sparse is throwing warnings when building sunrpc modules due to some
endianness shenanigans in ipv6.h. Sprinkle some endianness fixups to
silence them. These should all get fixed up at compile time, so I don't
think this will add any extra work to be done at runtime.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 include/net/ipv6.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 574337fe72dd..5ed2c24fe950 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -557,9 +557,9 @@ static inline u32 __ipv6_addr_jhash(const struct in6_addr *a, const u32 initval)
 static inline bool ipv6_addr_loopback(const struct in6_addr *a)
 {
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
-	const unsigned long *ul = (const unsigned long *)a;
+	const __be64 *be = (const __be64 *)a;
 
-	return (ul[0] | (ul[1] ^ cpu_to_be64(1))) == 0UL;
+	return (be[0] | (be[1] ^ cpu_to_be64(1))) == cpu_to_be64(0UL);
 #else
 	return (a->s6_addr32[0] | a->s6_addr32[1] |
 		a->s6_addr32[2] | (a->s6_addr32[3] ^ htonl(1))) == 0;
@@ -570,11 +570,11 @@ static inline bool ipv6_addr_v4mapped(const struct in6_addr *a)
 {
 	return (
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
-		*(__be64 *)a |
+		(*(__be64 *)a != cpu_to_be64(0)) |
 #else
-		(a->s6_addr32[0] | a->s6_addr32[1]) |
+		((a->s6_addr32[0] | a->s6_addr32[1]) != cpu_to_be32(0)) |
 #endif
-		(a->s6_addr32[2] ^ htonl(0x0000ffff))) == 0UL;
+		((a->s6_addr32[2] ^ htonl(0x0000ffff)) == cpu_to_be32(0)));
 }
 
 /*
-- 
1.9.3

Re: [PATCH] net: clean up some sparse endianness warnings in ipv6.h

[Christoph Hellwig]

>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
> -	const unsigned long *ul = (const unsigned long *)a;
> +	const __be64 *be = (const __be64 *)a;
>  
> -	return (ul[0] | (ul[1] ^ cpu_to_be64(1))) == 0UL;
> +	return (be[0] | (be[1] ^ cpu_to_be64(1))) == cpu_to_be64(0UL);

Do you need the swap for 0UL?  I know sparse treats 0 as special, so why
wouldn't it treat 0UL special?  Or just remove the 0UL postfix, no need
for it in a simple comparism.

Otherwise looks fine to me.

Re: [PATCH] net: clean up some sparse endianness warnings in ipv6.h

[Jeff Layton]

On Tue, 15 Jul 2014 10:01:07 -0700
Christoph Hellwig <hch@infradead.org> wrote:

> >  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
> > -	const unsigned long *ul = (const unsigned long *)a;
> > +	const __be64 *be = (const __be64 *)a;
> >  
> > -	return (ul[0] | (ul[1] ^ cpu_to_be64(1))) == 0UL;
> > +	return (be[0] | (be[1] ^ cpu_to_be64(1))) == cpu_to_be64(0UL);
> 
> Do you need the swap for 0UL?  I know sparse treats 0 as special, so why
> wouldn't it treat 0UL special?  Or just remove the 0UL postfix, no need
> for it in a simple comparism.
> 
> Otherwise looks fine to me.

Maybe not, I did it for completeness sake. I'll see if I can remove
that. The macros do the conversion at compile time though so it
shouldn't hurt anything either way.

-- 
Jeff Layton <jlayton@primarydata.com>

Re: [PATCH] net: clean up some sparse endianness warnings in ipv6.h

[Jeff Layton]

On Mon, 14 Jul 2014 08:25:46 -0400
Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> wrote:

> sparse is throwing warnings when building sunrpc modules due to some
> endianness shenanigans in ipv6.h. Sprinkle some endianness fixups to
> silence them. These should all get fixed up at compile time, so I don't
> think this will add any extra work to be done at runtime.
> 
> Signed-off-by: Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
> ---
>  include/net/ipv6.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 574337fe72dd..5ed2c24fe950 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -557,9 +557,9 @@ static inline u32 __ipv6_addr_jhash(const struct in6_addr *a, const u32 initval)
>  static inline bool ipv6_addr_loopback(const struct in6_addr *a)
>  {
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
> -	const unsigned long *ul = (const unsigned long *)a;
> +	const __be64 *be = (const __be64 *)a;
>  
> -	return (ul[0] | (ul[1] ^ cpu_to_be64(1))) == 0UL;
> +	return (be[0] | (be[1] ^ cpu_to_be64(1))) == cpu_to_be64(0UL);
>  #else
>  	return (a->s6_addr32[0] | a->s6_addr32[1] |
>  		a->s6_addr32[2] | (a->s6_addr32[3] ^ htonl(1))) == 0;
> @@ -570,11 +570,11 @@ static inline bool ipv6_addr_v4mapped(const struct in6_addr *a)
>  {
>  	return (
>  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
> -		*(__be64 *)a |
> +		(*(__be64 *)a != cpu_to_be64(0)) |
>  #else
> -		(a->s6_addr32[0] | a->s6_addr32[1]) |
> +		((a->s6_addr32[0] | a->s6_addr32[1]) != cpu_to_be32(0)) |
>  #endif
> -		(a->s6_addr32[2] ^ htonl(0x0000ffff))) == 0UL;
> +		((a->s6_addr32[2] ^ htonl(0x0000ffff)) == cpu_to_be32(0)));
>  }
>  
>  /*

Oof. I think I had the ipv6_addr_v4mapped changes wrong before due to
misreading the code. The existing code basically takes the different
values, does a bunch of bitwise operations on them and then compares
the result to 0.

My patch had it doing more than one comparison. I've got a fixed
version, but it does mean that we need to use a __force cast in one
place. I'll resend once I've tested it our some more.

Sorry for the noise!
-- 
Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: clean up some sparse endianness warnings in ipv6.h

[David Miller]

From: Jeff Layton <jlayton@primarydata.com>
Date: Mon, 14 Jul 2014 08:25:46 -0400

>  #endif
> -		(a->s6_addr32[2] ^ htonl(0x0000ffff))) == 0UL;
> +		((a->s6_addr32[2] ^ htonl(0x0000ffff)) == cpu_to_be32(0)));

While you are spinning a new version, change this htonl() to be
cpu_to_be32().  All the rest of this entire expression uses the
cpu_to_*() interfaces, so for consistency this should too.

Thanks.

[PATCH v2] net: clean up some sparse endianness warnings in ipv6.h

[Jeff Layton]

sparse is throwing warnings when building sunrpc modules due to some
endianness shenanigans in ipv6.h. Specifically:

  CHECK   net/sunrpc/addr.c
include/net/ipv6.h:573:17: warning: restricted __be64 degrades to integer
include/net/ipv6.h:577:34: warning: restricted __be32 degrades to integer
include/net/ipv6.h:573:17: warning: restricted __be64 degrades to integer
include/net/ipv6.h:577:34: warning: restricted __be32 degrades to integer

Sprinkle some endianness fixups to silence them. These should all get
fixed up at compile time, so I don't think this will add any extra work
to be done at runtime.

Signed-off-by: Jeff Layton <jlayton@primarydata.com>
---
 include/net/ipv6.h | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 574337fe72dd..0535da61fe0b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -557,24 +557,29 @@ static inline u32 __ipv6_addr_jhash(const struct in6_addr *a, const u32 initval)
 static inline bool ipv6_addr_loopback(const struct in6_addr *a)
 {
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
-	const unsigned long *ul = (const unsigned long *)a;
+	const __be64 *be = (const __be64 *)a;
 
-	return (ul[0] | (ul[1] ^ cpu_to_be64(1))) == 0UL;
+	return (be[0] | (be[1] ^ cpu_to_be64(1))) == 0UL;
 #else
 	return (a->s6_addr32[0] | a->s6_addr32[1] |
-		a->s6_addr32[2] | (a->s6_addr32[3] ^ htonl(1))) == 0;
+		a->s6_addr32[2] | (a->s6_addr32[3] ^ cpu_to_be32(1))) == 0;
 #endif
 }
 
+/*
+ * Note that we must __force cast these to unsigned long to make sparse happy,
+ * since all of the endian-annotated types are fixed size regardless of arch.
+ */
 static inline bool ipv6_addr_v4mapped(const struct in6_addr *a)
 {
 	return (
 #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
-		*(__be64 *)a |
+		*(unsigned long *)a |
 #else
-		(a->s6_addr32[0] | a->s6_addr32[1]) |
+		(__force unsigned long)(a->s6_addr32[0] | a->s6_addr32[1]) |
 #endif
-		(a->s6_addr32[2] ^ htonl(0x0000ffff))) == 0UL;
+		(__force unsigned long)(a->s6_addr32[2] ^
+					cpu_to_be32(0x0000ffff))) == 0UL;
 }
 
 /*
-- 
1.9.3

Re: [PATCH v2] net: clean up some sparse endianness warnings in ipv6.h

[David Miller]

From: Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
Date: Wed, 16 Jul 2014 06:55:46 -0400

> sparse is throwing warnings when building sunrpc modules due to some
> endianness shenanigans in ipv6.h. Specifically:
> 
>   CHECK   net/sunrpc/addr.c
> include/net/ipv6.h:573:17: warning: restricted __be64 degrades to integer
> include/net/ipv6.h:577:34: warning: restricted __be32 degrades to integer
> include/net/ipv6.h:573:17: warning: restricted __be64 degrades to integer
> include/net/ipv6.h:577:34: warning: restricted __be32 degrades to integer
> 
> Sprinkle some endianness fixups to silence them. These should all get
> fixed up at compile time, so I don't think this will add any extra work
> to be done at runtime.
> 
> Signed-off-by: Jeff Layton <jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>

Applied, thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html