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 <[email protected]>
---
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
On Mon, 14 Jul 2014 08:25:46 -0400
Jeff Layton <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
> #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.
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 <[email protected]>
---
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
On Tue, 15 Jul 2014 10:01:07 -0700
Christoph Hellwig <[email protected]> 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 <[email protected]>
From: Jeff Layton <[email protected]>
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.
From: Jeff Layton <[email protected]>
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 <[email protected]>
Applied, thank you.