* [PATCH] af_key: suppress a warning for 64k pages.
@ 2007-10-31 3:59 Stephen Rothwell
2007-10-31 4:08 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2007-10-31 3:59 UTC (permalink / raw)
To: netdev; +Cc: LKML
On PowerPC allmodconfig build we get this:
net/key/af_key.c:400: warning: comparison is always false due to limited range of data type
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
net/key/af_key.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 7969f8a..7da6c1a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -397,8 +397,11 @@ static inline int verify_sec_ctx_len(void *p)
struct sadb_x_sec_ctx *sec_ctx = (struct sadb_x_sec_ctx *)p;
int len;
+ /* sadb_x_ctx_len is uint16_t */
+#if PAGE_SIZE < (1 << 16)
if (sec_ctx->sadb_x_ctx_len > PAGE_SIZE)
return -EINVAL;
+#endif
len = pfkey_sec_ctx_len(sec_ctx);
--
1.5.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] af_key: suppress a warning for 64k pages.
2007-10-31 3:59 [PATCH] af_key: suppress a warning for 64k pages Stephen Rothwell
@ 2007-10-31 4:08 ` David Miller
2007-10-31 4:34 ` Stephen Rothwell
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2007-10-31 4:08 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-kernel
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 31 Oct 2007 14:59:54 +1100
> On PowerPC allmodconfig build we get this:
>
> net/key/af_key.c:400: warning: comparison is always false due to limited range of data type
>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
...
>
> + /* sadb_x_ctx_len is uint16_t */
> +#if PAGE_SIZE < (1 << 16)
> if (sec_ctx->sadb_x_ctx_len > PAGE_SIZE)
> return -EINVAL;
> +#endif
>
> len = pfkey_sec_ctx_len(sec_ctx);
>
I'm not so sure ifdef'ing things up all over the place is the way to
solve this. It makes the code ultra ugly.
I think we should retain the check, but modify it so that GCC knows we
understand that it's OK if it is always false. Perhaps a simple (u32)
cast on the left branch of the comparison is sufficient?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] af_key: suppress a warning for 64k pages.
2007-10-31 4:08 ` David Miller
@ 2007-10-31 4:34 ` Stephen Rothwell
2007-10-31 4:42 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2007-10-31 4:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
On Tue, 30 Oct 2007 21:08:46 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
> I'm not so sure ifdef'ing things up all over the place is the way to
> solve this. It makes the code ultra ugly.
I agree.
> I think we should retain the check, but modify it so that GCC knows we
> understand that it's OK if it is always false. Perhaps a simple (u32)
> cast on the left branch of the comparison is sufficient?
Unfortunately, that does not suppress the warning (gcc is getting too
smart :-().
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] af_key: suppress a warning for 64k pages.
2007-10-31 4:34 ` Stephen Rothwell
@ 2007-10-31 4:42 ` David Miller
2007-10-31 5:24 ` Stephen Rothwell
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2007-10-31 4:42 UTC (permalink / raw)
To: sfr; +Cc: netdev, linux-kernel
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 31 Oct 2007 15:34:22 +1100
> On Tue, 30 Oct 2007 21:08:46 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> > I think we should retain the check, but modify it so that GCC knows we
> > understand that it's OK if it is always false. Perhaps a simple (u32)
> > cast on the left branch of the comparison is sufficient?
>
> Unfortunately, that does not suppress the warning (gcc is getting too
> smart :-().
It seems if you break the comparison out into a function which
takes a u32, that's enough to get rid of the warning.
I can't figure out a way to make this prettier, can you?
#define PAGE_SIZE (64 * 1024)
typedef unsigned int u32;
typedef unsigned short u16;
int compare(u32 val)
{
if (val >= PAGE_SIZE)
return -1;
return 0;
}
int foo(u16 val)
{
#if 1
return compare(val);
#else
if (val >= PAGE_SIZE)
return -1;
return 0;
#endif
}
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] af_key: suppress a warning for 64k pages.
2007-10-31 4:42 ` David Miller
@ 2007-10-31 5:24 ` Stephen Rothwell
2007-10-31 6:58 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Stephen Rothwell @ 2007-10-31 5:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-kernel
On PowerPC allmodconfig build we get this:
net/key/af_key.c:400: warning: comparison is always false due to limited range of data type
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
net/key/af_key.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
This version gets rid of the warning without the ugliness of ifdefs.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 7969f8a..266f112 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -395,9 +395,9 @@ static inline int pfkey_sec_ctx_len(struct sadb_x_sec_ctx *sec_ctx)
static inline int verify_sec_ctx_len(void *p)
{
struct sadb_x_sec_ctx *sec_ctx = (struct sadb_x_sec_ctx *)p;
- int len;
+ int len = sec_ctx->sadb_x_ctx_len;
- if (sec_ctx->sadb_x_ctx_len > PAGE_SIZE)
+ if (len > PAGE_SIZE)
return -EINVAL;
len = pfkey_sec_ctx_len(sec_ctx);
--
1.5.3.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-31 6:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-31 3:59 [PATCH] af_key: suppress a warning for 64k pages Stephen Rothwell
2007-10-31 4:08 ` David Miller
2007-10-31 4:34 ` Stephen Rothwell
2007-10-31 4:42 ` David Miller
2007-10-31 5:24 ` Stephen Rothwell
2007-10-31 6:58 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox