Hello, all!
while compiling the kernel (2.6.1) I have spotted the following warning:
net/ipv6/ndisc.c: In function `ndisc_router_discovery':
net/ipv6/ndisc.c:1113: warning: comparison is always true due to limited range of data type
net/ipv6/ndisc.c:1121: warning: comparison is always true due to limited range of data type
The corresponding lines are these:
__u32 rtime = ntohl(ra_msg->retrans_timer);
Here ---> if (rtime && rtime/1000 < (MAX_SCHEDULE_TIMEOUT/HZ)) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
rtime = HZ/10;
in6_dev->nd_parms->retrans_time = rtime;
}
rtime = ntohl(ra_msg->reachable_time);
and here --> if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
rtime = (rtime*HZ)/1000;
The MAX_SCHEDULE_TIMEOUT is #defined to LONG_MAX in include/linux/sched.h,
which is 2^63-1 or so on AMD64. I propose the following fix:
--- net/ipv6/ndisc.c.orig 2004-01-29 22:03:50.553745520 +0100
+++ net/ipv6/ndisc.c 2004-01-29 22:06:39.973989728 +0100
@@ -995,6 +995,9 @@
}
}
+#define MAX_SCHEDULE_TIMEOUT_32 (MAX_SCHEDULE_TIMEOUT/HZ > (1U<<31) ? \
+ (1U<<31) : MAX_SCHEDULE_TIMEOUT/HZ)
+
static void ndisc_router_discovery(struct sk_buff *skb)
{
struct ra_msg *ra_msg = (struct ra_msg *) skb->h.raw;
@@ -1110,7 +1113,7 @@
if (in6_dev->nd_parms) {
__u32 rtime = ntohl(ra_msg->retrans_timer);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/HZ) {
+ if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT_32) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
rtime = HZ/10;
@@ -1118,7 +1121,7 @@
}
rtime = ntohl(ra_msg->reachable_time);
- if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT/(3*HZ)) {
+ if (rtime && rtime/1000 < MAX_SCHEDULE_TIMEOUT_32/3) {
rtime = (rtime*HZ)/1000;
if (rtime < HZ/10)
Do you think this is a correct fix? If so, please apply. Hope
this helps,
-Yenya
--
| Jan "Yenya" Kasprzak <kas at {fi.muni.cz - work | yenya.net - private}> |
| GPG: ID 1024/D3498839 Fingerprint 0D99A7FB206605D7 8B35FCDE05B18A5E |
| http://www.fi.muni.cz/~kas/ Czech Linux Homepage: http://www.linux.cz/ |
Any compiler or language that likes to hide things like memory allocations
behind your back just isn't a good choice for a kernel. --Linus Torvalds
In article <[email protected]> (at Thu, 29 Jan 2004 22:15:38 +0100), Jan Kasprzak <[email protected]> says:
> while compiling the kernel (2.6.1) I have spotted the following warning:
>
> net/ipv6/ndisc.c: In function `ndisc_router_discovery':
> net/ipv6/ndisc.c:1113: warning: comparison is always true due to limited range of data type
:
> The corresponding lines are these:
>
> __u32 rtime = ntohl(ra_msg->retrans_timer);
>
> Here ---> if (rtime && rtime/1000 < (MAX_SCHEDULE_TIMEOUT/HZ)) {
> rtime = (rtime*HZ)/1000;
> if (rtime < HZ/10)
> rtime = HZ/10;
> in6_dev->nd_parms->retrans_time = rtime;
> }
:
> The MAX_SCHEDULE_TIMEOUT is #defined to LONG_MAX in include/linux/sched.h,
> which is 2^63-1 or so on AMD64. I propose the following fix:
:
+#define MAX_SCHEDULE_TIMEOUT_32 (MAX_SCHEDULE_TIMEOUT/HZ > (1U<<31) ? \
+ (1U<<31) : MAX_SCHEDULE_TIMEOUT/HZ)
Well,... ok for now.
For long term solution, I think it is better to store timing variables
in "unsinged long" type instead of int.
I think there's several places to be fixed.
We'll need proc_doulongvec_jiffies(), proc_doulongvec_userhz_jiffies().
--yoshfuji
On Fri, 30 Jan 2004 08:37:43 +0900 (JST)
YOSHIFUJI Hideaki / 吉藤英明 <[email protected]> wrote:
> For long term solution, I think it is better to store timing variables
> in "unsinged long" type instead of int.
I think this is the only fix to even consider, even in the short term.
The macro suggested is just too much of a hack. :)
We can just ignore the silly warning until we are able to find time
to do the correct fix.
On Thu, 29 Jan 2004 15:39:53 -0800
"David S. Miller" <[email protected]> wrote:
> On Fri, 30 Jan 2004 08:37:43 +0900 (JST)
> YOSHIFUJI Hideaki / 吉藤英明 <[email protected]> wrote:
>
> > For long term solution, I think it is better to store timing variables
> > in "unsinged long" type instead of int.
>
> I think this is the only fix to even consider, even in the short term.
> The macro suggested is just too much of a hack. :)
>
> We can just ignore the silly warning until we are able to find time
> to do the correct fix.
Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
FWIW only IPv6, keyboard driver and ACPI are spewing warnings on x86-64 in a defconfig
compile.
-Andi
On Fri, 30 Jan 2004 01:40:31 +0100
Andi Kleen <[email protected]> wrote:
> Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
Nope, for some reason gcc-3.2.3 on my systems is missing it.
Otherwise I would have killed this earlier :)
On Thu, 29 Jan 2004 16:56:45 -0800
"David S. Miller" <[email protected]> wrote:
> On Fri, 30 Jan 2004 01:40:31 +0100
> Andi Kleen <[email protected]> wrote:
>
> > Fine by me. I've been ignoring it forever. But don't you see it on sparc64 too?
>
> Nope, for some reason gcc-3.2.3 on my systems is missing it.
gcc 3.3 is printing it.
-Andi