Re: [linux-fbdev] gcc -W for 2.2.17-pre14
On Tue, 1 Aug 2000, Andrew Morton wrote:
drivers/video/fbmem.c:478
if (con2fb.console 0 || con2fb.console MAX_NR_CONSOLES)
return -EINVAL;
I think this is an off-by-one. Should it be `= MAX_NR_CONSOLES'?
No, see the code below.
0 means `set them all', 1--MAX_NR_CONSOLES means `set VC i'.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [EMAIL PROTECTED]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
gcc -W for 2.2.17-pre14
Feeding kernel 2.2.17 through `gcc -W' has shown a number of bugs.
I have attached here a proposed patch which fixes a small number of these,
but for the rest I would ask those on the "To:" list to let me know how they
would prefer that these be addressed. I shall then cut a final patch.
Thanks.
acenic.c:973: warning: unsigned value 0 is always 0
acenic.c:983: warning: unsigned value 0 is always 0
Error returns from read_eeprom_byte() are not recognised. This
buglet is also present in the 2.4 driver. No patch provided here -
I'll assume that Jes will be sending one through when he updates the
2.4 driver.
hmm.. acenic doesn't have the other fixes applied yet:
The precedence error in ace_timer (5/2*HZ).
dev-priv should be zeroed after allocation (there was a crash
codepath without this).
del_timer_sync() for 2.4.
af_ax25.c:519: warning: unsigned value 0 is always 0
af_ax25.c:525: warning: unsigned value 0 is always 0
Bounds checking on ioctl args. Probably not worth fixing.
b1.c:601: warning: unsigned value = 0 is always 1
b1.c:612: warning: unsigned value = 0 is always 1
b1dma.c:556: warning: unsigned value = 0 is always 1
b1dma.c:567: warning: unsigned value = 0 is always 1
c4.c:659: warning: unsigned value = 0 is always 1
c4.c:670: warning: unsigned value = 0 is always 1
t1isa.c:283: warning: unsigned value = 0 is always 1
t1isa.c:294: warning: unsigned value = 0 is always 1
Potentially nasty - underindexing of card-MsgBuf[]. Patched here.
br.c:2261: warning: unsigned value 0 is always 0
br.c:2279: warning: unsigned value 0 is always 0
br.c:2302: warning: unsigned value 0 is always 0
br.c:2313: warning: unsigned value 0 is always 0
Nasty - can result in underindexing of port_info[]. Unprivileged
users can possibly crash the machine. Patched here.
cs46xx.c:2215: warning: unsigned value = 0 is always 1
cs46xx.c:2246: warning: unsigned value = 0 is always 1
These are potentially nasty. A hardware-bit-testing loop which
is supposed to have a timeout is in fact potentially infinite.
No patch here because it would be behaviour-altering and I can't
test it.
The fix is to make `end_time' signed. Should use time_after().
epca.c:3836: warning: ordered comparison of pointer with integer zero
epca.c:3848: warning: ordered comparison of pointer with integer zero
I'm not sure what `pointer = 0' does... But this is a simple
fix to the input parameter validation and I've patched it here.
drivers/video/fbmem.c:478
if (con2fb.console 0 || con2fb.console MAX_NR_CONSOLES)
return -EINVAL;
I think this is an off-by-one. Should it be `= MAX_NR_CONSOLES'?
hp100.c:2920: warning: comparison of promoted ~unsigned with constant
This code:
do {
if (~(hp100_inb( VG_LAN_CFG_1 ) HP100_LINK_UP_ST) ) break;
} while (time_after(time, jiffies));
Is clearly bogus (the `~'). But a fix is behaviour-altering and I
can't test it.
./ip2/i2lib.c:1261: warning: comparisons like X=Y=Z do not have their
mathematical meaning
It looks like this code flukes correctness.
iph5526.c:3778: warning: comparisons like X=Y=Z do not have their
mathematical meaning
Ditto.
ixj.c:3029: warning: empty body in an if-statement
if (ixj[board].play_mode != -1 ixj[board].rec_mode != -1);
{
ixj_WriteDSPCommand(0xB002, board); // AEC Stop
}
See the semicolon? This has to be a bug, but a fix is
behaviour-altering and I can't test it.
ll_rw_blk.c:761: warning: unsigned value 0 is always 0
Here's the code:
/* Can we add it to the end of this request? */
if (req-sector + req-nr_sectors == sector) {
if (latency - req-nr_segments 0)
break;
The inner test is a no-op. I think we decided it could simply
be removed in the 2.4 case. Is that true here as well?
drivers/sound/emu10k1/main.c:653: warning: unsigned value 0 is always 0
drivers/sound/emu10k1/main.c:658: warning: unsigned value 0 is always 0
drivers/sound/emu10k1/main.c:663: warning: unsigned value 0 is always 0
drivers/sound/emu10k1/main.c:668: warning: unsigned value 0 is always 0
Four ineffective attempts to check return values.
mm/memory.c:395: warning: unsigned value 0 is always 0
Here's the code:
if (mm-rss 0) {
mm-rss -= freed;
if (mm-rss 0)
mm-rss = 0;
}
The check for RSS underflow is ineffective because it's unsigned.
What are we actually trying to do here?
net/netrom/nr_route.c:625
if (nr_route.ndigis 0 || nr_route.ndigis AX25_MAX_DIGIS)
return -EINVAL;
Is this an off-by-one error? Should it be `= AX25_MAX_DIGIS'?
old_tulip.c:2732: warning: unsigned value = 0 is always 1
hmmm.. In 2.4 this was a bug because `dummy' was initialised to
-1. Here it's initialised to zero and
