RE: bugs in /usr/src/linux/net/ipv6/mcast.c

RE: bugs in /usr/src/linux/net/ipv6/mcast.c

[Cipriani, Lawrence V (Larry)]

Thanks.  We're on 2.4.x  For what it's worth, here are a few more I found:

/usr/src/linux/drivers/sound/vidc.c: extra semicolon near line 227

for (new2size = 128; new2size < newsize; new2size <<= 1);  	!!!
        if (new2size - newsize > newsize - (new2size >> 1))
                new2size >>= 1;                            

-----------------------------------------------------------
/usr/src/linux/sound/drivers/serialmidi.c: extra semicolon near line 441

        if (serial->sdev);                             !!!
                kfree(serial->sdev);                   

-----------------------------------------------------------

/usr/src/linux/drivers/s390/misc/chandev.c: extra semicolon near line 2031

	if(chandev_find_eligible_channels(curr_chandev,
		&read_chandev,
		&write_chandev,
		&data_chandev,
		&next_chandev,
		curr_force->chan_type));			!!!
	{                                               

-----------------------------------------------------------

/usr/src/linux/drivers/s390/misc/chandev.c: extra semicolong near line 2150

while(!atomic_compare_and_swap(TRUE,FALSE,&chandev_new_msck));		!!!
{                                                             
        chandev_probe();                                      
}                                                             

-----------------------------------------------------------

/usr/src/linux/drivers/s390/block/dasd.c: extra semicolon near line 1528

		if (device->discipline->term_IO (cqr) != 0);  !!!
			cqr->status = CQR_STATUS_FAILED;

-----------------------------------------------------------

/usr/src/linux/drivers/scsi/ide-scsi.c: extra semicolon near line 855

	for (id = 0;
	        id < MAX_HWIFS*MAX_DRIVES && idescsi_drives[id];
	                    id++);		!!!
	        idescsi_setup(drive, scsi, id);

-----------------------------------------------------------
/usr/src/linux/drivers/scsi/osst.c: extra semicolon near line 5264

                for (nbr=0; osst_buffers[nbr] != STbuffer && nbr < osst_nbr_buffers; nbr++);   	!!!
                        printk(OSST_DEB_MSG
                           "osst :D: Expanded tape buffer %d (%d bytes, %d->%d segments, dma: %d, a: %p).\n",
                           nbr, got, STbuffer->orig_sg_segs, STbuffer->sg_segs, need_dma, STbuffer->b_data);
                        printk(OSST_DEB_MSG
                           "osst :D: segment sizes: first %d, last %d bytes.\n",
                           STbuffer->sg[0].length, STbuffer->sg[segs-1].length);

-----------------------------------------------------------

I'm not sure about this one, it sure looks hinky:

/usr/src/linux/abi/svr4/misc.c: extra semicolon near line 564:

for (p = tmp; *p; p++); 		!!!
        p--;            

might need to be:

for (p = tmp; *p; p++)
        p--;            

-----------------------------------------------------------

My C/C++ static analyzer reported several other things too, I'll send them in later.

Larry

-----Original Message-----
From: David S. Miller [mailto:davem@davemloft.net]
Sent: Wednesday, November 16, 2005 4:02 PM
To: lvc@lucent.com
Cc: linux-kernel@vger.kernel.org
Subject: Re: bugs in /usr/src/linux/net/ipv6/mcast.c


From: "Cipriani, Lawrence V (Larry)" <lvc@lucent.com>
Date: Wed, 16 Nov 2005 09:53:07 -0500

> /usr/src/linux/net/ipv6/mcast.c: extra semicolon near line 609         
>                 if (mc->sfmode == MCAST_INCLUDE && i >= psl->sl_count);
>                         rv = 0;                                        
> should be:
> 		    if (mc->sfmode == MCAST_EXCLUDE && i >= psl->sl_count)
> 				rv = 0;
> 
> /usr/src/linux/net/ipv6/mcast.c: extra semicolon near line 611         
>                 if (mc->sfmode == MCAST_EXCLUDE && i < psl->sl_count); 
>                         rv = 0;                             
> should be:
> 		    if (mc->sfmode == MCAST_EXCLUDE && i < psl->sl_count)
> 				rv = 0;

These have been fixed for a while now in 2.6.x

[PATCH] Re: bugs in /usr/src/linux/net/ipv6/mcast.c

[Denis Vlasenko]

[-- Attachment #1: Type: text/plain, Size: 506 bytes --]

[CC: added to respective maintainers]

On Thursday 17 November 2005 00:24, Cipriani, Lawrence V (Larry) wrote:
> 
> Thanks.  We're on 2.4.x  For what it's worth, here are a few more I found:

As far as I can see, none of these exist in 2.6.

(BTW, how did you find them? That one with multi line 'while'
is hard to find with grep)

However, a few similar bugs do exist in 2.6!

Patch attached.

Patch intentionally places a comment instead of statement
in few false positives.

Please review/apply.
--
vda

[-- Attachment #2: linux-2.6.14.semicolon_fix.diff --]
[-- Type: text/x-diff, Size: 3334 bytes --]

diff -urpN linux-2.6.14.org/drivers/isdn/capi/capiutil.c linux-2.6.14.semicolon_fix/drivers/isdn/capi/capiutil.c
--- linux-2.6.14.org/drivers/isdn/capi/capiutil.c	Mon Aug 29 02:41:01 2005
+++ linux-2.6.14.semicolon_fix/drivers/isdn/capi/capiutil.c	Thu Nov 17 14:41:58 2005
@@ -404,7 +404,8 @@ static unsigned command_2_index(unsigned
 {
 	if (c & 0x80)
 		c = 0x9 + (c & 0x0f);
-	else if (c <= 0x0f);
+	else if (c <= 0x0f)
+		/* do nothing */;
 	else if (c == 0x41)
 		c = 0x9 + 0x1;
 	else if (c == 0xff)
diff -urpN linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c
--- linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c	Sat Nov  5 15:17:30 2005
+++ linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c	Thu Nov 17 14:41:05 2005
@@ -140,25 +140,25 @@ static int ves1820_set_symbolrate(struct
 	/* yeuch! */
 	fpxin = state->config->xin * 10;
 	fptmp = fpxin; do_div(fptmp, 123);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 160);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 246);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 320);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 492);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 640);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 984);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 
 	fin = state->config->xin >> 4;
diff -urpN linux-2.6.14.org/drivers/net/tokenring/ibmtr.c linux-2.6.14.semicolon_fix/drivers/net/tokenring/ibmtr.c
--- linux-2.6.14.org/drivers/net/tokenring/ibmtr.c	Sat Nov  5 15:17:37 2005
+++ linux-2.6.14.semicolon_fix/drivers/net/tokenring/ibmtr.c	Thu Nov 17 14:42:56 2005
@@ -610,8 +610,10 @@ static int __devinit ibmtr_probe1(struct
 	ti->mapped_ram_size= /*sixteen to onehundredtwentyeight 512byte blocks*/
 	    1<< ((readb(ti->mmio+ACA_OFFSET+ACA_RW+RRR_ODD) >> 2 & 0x03) + 4);
 	ti->page_mask = 0;
-	if (ti->turbo)  ti->page_mask=0xf0;
-	else if (ti->shared_ram_paging == 0xf);  /* No paging in adapter */
+	if (ti->turbo)
+		ti->page_mask=0xf0;
+	else if (ti->shared_ram_paging == 0xf)
+		/* No paging in adapter */;
 	else {
 #ifdef ENABLE_PAGING
 		unsigned char pg_size = 0;
diff -urpN linux-2.6.14.org/net/netrom/nr_in.c linux-2.6.14.semicolon_fix/net/netrom/nr_in.c
--- linux-2.6.14.org/net/netrom/nr_in.c	Sat Nov  5 15:18:17 2005
+++ linux-2.6.14.semicolon_fix/net/netrom/nr_in.c	Thu Nov 17 14:43:53 2005
@@ -99,7 +99,7 @@ static int nr_state1_machine(struct sock
 		break;
 
 	case NR_RESET:
-		if (sysctl_netrom_reset_circuit);
+		if (sysctl_netrom_reset_circuit)
 			nr_disconnect(sk, ECONNRESET);
 		break;
 
@@ -130,7 +130,7 @@ static int nr_state2_machine(struct sock
 		break;
 
 	case NR_RESET:
-		if (sysctl_netrom_reset_circuit);
+		if (sysctl_netrom_reset_circuit)
 			nr_disconnect(sk, ECONNRESET);
 		break;
 
@@ -265,7 +265,7 @@ static int nr_state3_machine(struct sock
 		break;
 
 	case NR_RESET:
-		if (sysctl_netrom_reset_circuit);
+		if (sysctl_netrom_reset_circuit)
 			nr_disconnect(sk, ECONNRESET);
 		break;
 

Re: [linux-dvb-maintainer] [PATCH] Re: bugs in /usr/src/linux/net/ipv6/mcast.c

[Johannes Stezenbach]

On Thu, Nov 17, 2005 Denis Vlasenko wrote:
> However, a few similar bugs do exist in 2.6!
> 
> Patch attached.
> 
> Patch intentionally places a comment instead of statement
> in few false positives.
> 
> Please review/apply.

Thanks a lot. /me wonders how this could go unnoticed for so long...

Can you add your Signed-off-by: ?

Johannes

> diff -urpN linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c
> --- linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c	Sat Nov  5 15:17:30 2005
> +++ linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c	Thu Nov 17 14:41:05 2005
> @@ -140,25 +140,25 @@ static int ves1820_set_symbolrate(struct
>  	/* yeuch! */
>  	fpxin = state->config->xin * 10;
>  	fptmp = fpxin; do_div(fptmp, 123);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 160);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 246);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 320);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 492);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 640);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 984);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  
>  	fin = state->config->xin >> 4;

Re: [linux-dvb-maintainer] [PATCH] Re: bugs in /usr/src/linux/net/ipv6/mcast.c

[Denis Vlasenko]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

On Sunday 20 November 2005 04:01, Johannes Stezenbach wrote:
> Can you add your Signed-off-by: ?

Sure.

Signed-off-by: Denis Vlasenko <vda@ilport.com.ua>
--
vda

[-- Attachment #2: linux-2.6.14.semicolon_fix.dvb.diff --]
[-- Type: text/x-diff, Size: 1178 bytes --]

diff -urpN linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c
--- linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c	Sat Nov  5 15:17:30 2005
+++ linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c	Thu Nov 17 14:41:05 2005
@@ -140,25 +140,25 @@ static int ves1820_set_symbolrate(struct
 	/* yeuch! */
 	fpxin = state->config->xin * 10;
 	fptmp = fpxin; do_div(fptmp, 123);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 160);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 246);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 320);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 492);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 	fptmp = fpxin; do_div(fptmp, 640);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 0;
 	fptmp = fpxin; do_div(fptmp, 984);
-	if (symbolrate < fptmp);
+	if (symbolrate < fptmp)
 		SFIL = 1;
 
 	fin = state->config->xin >> 4;

Re: [linux-dvb-maintainer] [PATCH] Re: bugs in /usr/src/linux/net/ipv6/mcast.c

[Johannes Stezenbach]

On Sun, Nov 20, 2005 at 01:27:23PM +0200, Denis Vlasenko wrote:
> On Sunday 20 November 2005 04:01, Johannes Stezenbach wrote:
> > Can you add your Signed-off-by: ?
> 
> Sure.
> 
> Signed-off-by: Denis Vlasenko <vda@ilport.com.ua>

Thanks, I added this patch to linuxtv.org CVS.

We'll see if this fix actually improves or breaks the driver ;-/

Johannes

> diff -urpN linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c
> --- linux-2.6.14.org/drivers/media/dvb/frontends/ves1820.c	Sat Nov  5 15:17:30 2005
> +++ linux-2.6.14.semicolon_fix/drivers/media/dvb/frontends/ves1820.c	Thu Nov 17 14:41:05 2005
> @@ -140,25 +140,25 @@ static int ves1820_set_symbolrate(struct
>  	/* yeuch! */
>  	fpxin = state->config->xin * 10;
>  	fptmp = fpxin; do_div(fptmp, 123);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 160);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 246);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 320);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 492);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  	fptmp = fpxin; do_div(fptmp, 640);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 0;
>  	fptmp = fpxin; do_div(fptmp, 984);
> -	if (symbolrate < fptmp);
> +	if (symbolrate < fptmp)
>  		SFIL = 1;
>  
>  	fin = state->config->xin >> 4;

Re: bugs in /usr/src/linux/net/ipv6/mcast.c

[Denis Vlasenko]

On 2.6.14 in drivers/isdn/hisax:

# grep -r '[^a-z0-9_]if *([^()]*([^)]*)[^)]*);' .
./hfc_sx.c:     if (Read_hfc(cs, HFCSX_INT_S1));
./hfc_sx.c:     if (Read_hfc(cs, HFCSX_INT_S2));
./hfc_sx.c:                                             if (Read_hfc(cs, HFCSX_INT_S1));
./hfc_pci.c:    if (Read_hfc(cs, HFCPCI_INT_S1));
./hfc_pci.c:    if (Read_hfc(cs, HFCPCI_INT_S1));
./hfc_pci.c:                                            if (Read_hfc(cs, HFCPCI_INT_S1));

These are not bugs, but rather "interesting" coding style:

        Write_hfc(cs, HFCSX_INT_M1, cs->hw.hfcsx.int_m1);

        /* Clear already pending ints */
        if (Read_hfc(cs, HFCSX_INT_S1));

        Write_hfc(cs, HFCSX_STATES, HFCSX_LOAD_STATE | 2);      /* HFC ST 2 */

Obviously author tried to silence something like lint.
I think it may be replaced with (void)expr; construct.
--
vda