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 the logic is subtly different.
   Jeff, is this driver broken?


drivers/net/sbni.c:1379: warning: control reaches end of non-void function

  This looks broken.  Why is the (required) `return crc' commented out?


mm/slab.c:717:

        if (offset < 0 || offset > size) {
                printk("%sOffset weird %d - %s\n", func_nm, (int) offset, name);
                offset = 0;
        }

  The first part of the comparison is ineffective when size_t is unsigned.

  The second part of the comparison seems to be an off-by-one.  Should it
  be `offset >= size'?


fs/ntfs/support.c:301: warning: `ch' might be used uninitialized in this
function
fs/ntfs/support.c:301: warning: `cl' might be used uninitialized in this
function

  The warning is correct.  This code looks buggy.


fs/coda/upcall.c:741: warning: unsigned value < 0 is always 0

  Ineffective attempt to check an error code.
--- linux-2.2.17pre14/drivers/isdn/avmb1/b1.c   Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/b1.c  Mon Jul 31 22:28:22 2000
@@ -507,7 +507,7 @@
        struct sk_buff *skb;
 
        unsigned ApplId;
-       unsigned MsgLen;
+       signed MsgLen;
        unsigned DataB3Len;
        unsigned NCCI;
        unsigned WindowSize;
--- linux-2.2.17pre14/drivers/isdn/avmb1/b1dma.c        Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/b1dma.c       Mon Jul 31 22:29:48 2000
@@ -462,7 +462,8 @@
        struct capi_ctr *ctrl = cinfo->capi_ctrl;
        struct sk_buff *skb;
        void *p = dma->recvbuf+4;
-       __u32 ApplId, MsgLen, DataB3Len, NCCI, WindowSize;
+       __u32 ApplId, DataB3Len, NCCI, WindowSize;
+       __s32 MsgLen;
        __u8 b1cmd =  _get_byte(&p);
 
 #ifdef CONFIG_B1DMA_DEBUG
--- linux-2.2.17pre14/net/bridge/br.c   Wed Jan  5 05:12:26 2000
+++ linux-akpm/net/bridge/br.c  Mon Jul 31 22:32:14 2000
@@ -2258,7 +2258,7 @@
                                        break;
                                case BRCMD_IF_ENABLE:
                                        bcf.arg1 = br_find_port(bcf.arg1);
-                                       if (bcf.arg1 < 0)
+                                       if ((signed)bcf.arg1 < 0)
                                                return(bcf.arg1);
                                case BRCMD_PORT_ENABLE:
                                        if (port_info[bcf.arg1].dev == 0)
@@ -2276,7 +2276,7 @@
                                        break;
                                case BRCMD_IF_DISABLE:
                                        bcf.arg1 = br_find_port(bcf.arg1);
-                                       if (bcf.arg1 < 0)
+                                       if ((signed)bcf.arg1 < 0)
                                                return(bcf.arg1);
                                case BRCMD_PORT_DISABLE:
                                        if (port_info[bcf.arg1].dev == 0)
@@ -2299,7 +2299,7 @@
                                        break;
                                case BRCMD_SET_IF_PRIORITY:
                                        bcf.arg1 = br_find_port(bcf.arg1);
-                                       if (bcf.arg1 < 0)
+                                       if ((signed)bcf.arg1 < 0)
                                                return(bcf.arg1);
                                case BRCMD_SET_PORT_PRIORITY:
                                        if((port_info[bcf.arg1].dev == 0)
@@ -2310,7 +2310,7 @@
                                        break;
                                case BRCMD_SET_IF_PATH_COST:
                                        bcf.arg1 = br_find_port(bcf.arg1);
-                                       if (bcf.arg1 < 0)
+                                       if ((signed)bcf.arg1 < 0)
                                                return(bcf.arg1);
                                case BRCMD_SET_PATH_COST:
                                        if (port_info[bcf.arg1].dev == 0)
--- linux-2.2.17pre14/drivers/isdn/avmb1/c4.c   Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/c4.c  Mon Jul 31 22:46:01 2000
@@ -535,7 +535,8 @@
        avmctrl_info *cinfo;
        struct sk_buff *skb;
        void *p = dma->recvbuf;
-       __u32 ApplId, MsgLen, DataB3Len, NCCI, WindowSize;
+       __u32 ApplId, DataB3Len, NCCI, WindowSize;
+       __s32 MsgLen;
        __u8 b1cmd =  _get_byte(&p);
        __u32 cidx;
 
--- linux-2.2.17pre14/drivers/char/epca.c       Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/char/epca.c      Mon Jul 31 22:56:57 2000
@@ -3833,7 +3833,7 @@
 
                        case 5:
                                board.port = (unsigned char *)ints[index];
-                               if (board.port <= 0)
+                               if ((signed long)board.port <= 0)
                                {
                                        printk(KERN_ERR "<Error> - epca_setup: Invalid 
io port 0x%x\n", (unsigned int)board.port);
                                        invalid_lilo_config = 1;
@@ -3845,7 +3845,7 @@
 
                        case 6:
                                board.membase = (unsigned char *)ints[index];
-                               if (board.membase <= 0)
+                               if ((signed long)board.membase <= 0)
                                {
                                        printk(KERN_ERR "<Error> - epca_setup: Invalid 
memory base 0x%x\n",(unsigned int)board.membase);
                                        invalid_lilo_config = 1;
--- linux-2.2.17pre14/drivers/isdn/avmb1/t1isa.c        Fri Jun 16 23:47:49 2000
+++ linux-akpm/drivers/isdn/avmb1/t1isa.c       Tue Aug  1 00:06:18 2000
@@ -190,7 +190,7 @@
        struct sk_buff *skb;
 
        unsigned ApplId;
-       unsigned MsgLen;
+       signed MsgLen;
        unsigned DataB3Len;
        unsigned NCCI;
        unsigned WindowSize;

Reply via email to