More compile warning fixes for 2.4.0

Here's a patch that fixes more of the compile warnings with gcc 2.97.

diff -urN -X dontdiff linux/drivers/atm/fore200e.c rb/drivers/atm/fore200e.c

@@ -437,7 +437,7 @@
 	/* XXX shouldn't we *start* by deregistering the device? */
 	atm_dev_deregister(fore200e->atm_dev);
 
-    case FORE200E_STATE_BLANK:
+    case FORE200E_STATE_BLANK:;
 	/* nothing to do for that state */
     }
 }

diff -urN -X dontdiff linux/drivers/atm/iphase.c rb/drivers/atm/iphase.c
@@ -951,7 +951,7 @@
                                   SUNI_PM7345_PLB);
 #ifdef __SNMP__
    suni_pm7345->suni_rxcp_intr_en_sts |= SUNI_OOCDE;
-#endif __SNMP__
+#endif /* __SNMP__ */
    return;
 }
 

diff -urN -X dontdiff linux/drivers/cdrom/sbpcd.c rb/drivers/cdrom/sbpcd.c
@@ -338,7 +338,7 @@
 
 #ifndef SBPCD_ISSUE
 #define SBPCD_ISSUE 1
-#endif SBPCD_ISSUE
+#endif /* SBPCD_ISSUE */
 
 #include <linux/module.h>
 

@@ -408,7 +408,7 @@
 #else
 #define SBPCD_CLI
 #define SBPCD_STI
-#endif SBPCD_DIS_IRQ
+#endif /* SBPCD_DIS_IRQ */
 /*==========================================================================*/
 /*
  * auto-probing address list

@@ -466,9 +466,9 @@
 	0x370, 0, /* Lasermate, CI-101P */
 	0x290, 1, /* Soundblaster 16 */
 	0x310, 0, /* Lasermate, CI-101P, WDH-7001C */
-#endif MODULE
+#endif /* MODULE */
 #endif
-#endif DISTRIBUTION
+#endif /* DISTRIBUTION */
 };
 #else
 static int sbpcd[] = {CDROM_PORT, SBPRO}; /* probe with user's setup only */

@@ -557,7 +557,7 @@
 			  (1<<DBG_TOC) |
 			  (1<<DBG_MUL) |
 			  (1<<DBG_UPC));
-#endif DISTRIBUTION
+#endif /* DISTRIBUTION */
 
 static int sbpcd_ioaddr = CDROM_PORT;	/* default I/O base address */
 static int sbpro_type = SBPRO;

@@ -606,7 +606,7 @@
 
 #if FUTURE
 static DECLARE_WAIT_QUEUE_HEAD(sbp_waitq);
-#endif FUTURE
+#endif /* FUTURE */
 
 static int teac=SBP_TEAC_SPEED;
 static int buffers=SBP_BUFFER_FRAMES;

@@ -631,7 +631,7 @@
 #if OLD_BUSY
 static volatile u_char busy_data;
 static volatile u_char busy_audio; /* true semaphores would be safer */
-#endif OLD_BUSY
+#endif /* OLD_BUSY */
 static DECLARE_MUTEX(ioctl_read_sem);
 static u_long timeout;
 static volatile u_char timed_out_delay;

@@ -648,7 +648,7 @@
 static u_int maxtim_data= 9000;
 #else
 static u_int maxtim_data= 3000;
-#endif LONG_TIMING
+#endif /* LONG_TIMING */
 #if DISTRIBUTION
 static int n_retries=6;
 #else

@@ -713,7 +713,7 @@
 	u_char vol_ctrl2;
 	char vol_chan3;
 	u_char vol_ctrl3;
-#endif 000
+#endif /* 000 */
 	u_char volume_control; /* TEAC on/off bits */
 	
 	u_char SubQ_ctl_adr;

@@ -742,7 +742,7 @@
 	u_int TocEnt_address;
 #if SAFE_MIXED
 	char has_data;
-#endif SAFE_MIXED
+#endif /* SAFE_MIXED */
 	u_char ored_ctl_adr; /* to detect if CDROM contains data tracks */
 	
 	struct {

@@ -792,7 +792,7 @@
 #define MSG_LEVEL KERN_NOTICE
 #else
 #define MSG_LEVEL KERN_INFO
-#endif DISTRIBUTION
+#endif /* DISTRIBUTION */
 
 	char buf[256];
 	va_list args;

@@ -808,7 +808,7 @@
 	printk(buf);
 #if KLOGD_PAUSE
 	sbp_sleep(KLOGD_PAUSE); /* else messages get lost */
-#endif KLOGD_PAUSE
+#endif /* KLOGD_PAUSE */
 	return;
 }
 /*==========================================================================*/

@@ -1102,7 +1102,7 @@
 	return (0);
 }
 /*==========================================================================*/
-#endif 00000
+#endif /* 00000 */
 /*==========================================================================*/
 static int ResponseInfo(void)
 {

@@ -1132,7 +1132,7 @@
 	}
 	j=i-response_count;
 	if (j>0) msg(DBG_INF,"ResponseInfo: got %d trailing bytes.\n",j);
-#endif 000
+#endif /* 000 */
 	for (j=0;j<i;j++)
 		sprintf(&msgbuf[j*3]," %02X",infobuf[j]);
 	msgbuf[j*3]=0;

@@ -1384,7 +1384,7 @@
 		if (drvcmd[0]==CMDT_READ_VER) sbp_sleep(HZ); /* fixme */
 #if 01
 		OUT(CDo_sel_i_d,1);
-#endif 01
+#endif /* 01 */
 		if (teac==2)
                   {
                     if ((i=CDi_stat_loop_T()) == -1) break;

@@ -1393,7 +1393,7 @@
                   {
 #if 0
                     OUT(CDo_sel_i_d,1);
-#endif 0
+#endif /* 0 */
                     i=inb(CDi_status);
                   }
 		if (!(i&s_not_data_ready)) /* f.e. CMDT_DISKINFO */

@@ -1417,7 +1417,7 @@
                                                         l=1;
                                                         msg(DBG_TEA,"cmd_out_T: do_16bit: false fi

rst byte!\n");
                                                 }
-#endif TEST_FALSE_FF
+#endif /* TEST_FALSE_FF */
                                         }
                                         else infobuf[l++]=inb(CDi_data);
                                         i=inb(CDi_status);

@@ -2012,7 +2012,7 @@
msg(DBG_TEA, "================CMDT_RESET given=================.\n"); sbp_sleep(3*HZ);
}
-#endif 1
+#endif /* 1 */
flush_status();
i=GetStatus();
if (i<0) return i;
@@ -2333,7 +2333,7 @@
i=ResponseStatus();

 #if 0
                 sbp_sleep(HZ);

-#endif 0
+#endif /* 0 */
i=ResponseStatus();
}
if (i<0)
@@ -2678,7 +2678,7 @@
D_S[d].vol_ctrl2=0xFF;
D_S[d].vol_chan3=3;
D_S[d].vol_ctrl3=0xFF;
-#endif 000
+#endif /* 000 */
D_S[d].diskstate_flags |= volume_bit;
return (0);
}
@@ -2978,20 +2978,20 @@
int i;
#if TEST_UPC
int block, checksum;
-#endif TEST_UPC
+#endif /* TEST_UPC */

if (fam2_drive) return (0); /* not implemented yet */
if (famT_drive) return (0); /* not implemented yet */
if (famV_drive) return (0); /* not implemented yet */
#if 1
if (fam0_drive) return (0); /* but it should work */
-#endif 1
+#endif /* 1 */

D_S[d].diskstate_flags &= ~upc_bit;
#if TEST_UPC
for (block=CD_MSF_OFFSET+1;block<CD_MSF_OFFSET+200;block++)
{
-#endif TEST_UPC
+#endif /* TEST_UPC */
clr_cmdbuf();
if (fam1_drive)
{
@@ -3000,7 +3000,7 @@
drvcmd[1]=(block>>16)&0xFF;
drvcmd[2]=(block>>8)&0xFF;
drvcmd[3]=block&0xFF;
-#endif TEST_UPC
+#endif /* TEST_UPC */
response_count=8; flags_cmd_out=f_putcmd|f_ResponseStatus|f_obey_p_check;
}
@@ -3011,7 +3011,7 @@
drvcmd[2]=(block>>16)&0xFF;
drvcmd[3]=(block>>8)&0xFF;
drvcmd[4]=block&0xFF;
-#endif TEST_UPC
+#endif /* TEST_UPC */
response_count=0; flags_cmd_out=f_putcmd|f_lopsta|f_getsta|f_ResponseStatus|f_obey_p_check|f_bit1;
}
@@ -3042,12 +3042,12 @@
}
#if TEST_UPC
checksum=0;
-#endif TEST_UPC
+#endif /* TEST_UPC */
for (i=0;i<(fam1_drive?8:16);i++)
{
#if TEST_UPC
checksum |= infobuf[i];
-#endif TEST_UPC
+#endif /* TEST_UPC */
sprintf(&msgbuf[i*3], " %02X", infobuf[i]);
}
msgbuf[i*3]=0;
@@ -3055,7 +3055,7 @@
#if TEST_UPC
if ((checksum&0x7F)!=0) break;
}
-#endif TEST_UPC
+#endif /* TEST_UPC */
D_S[d].UPC_ctl_adr=0;
if (fam1_drive) i=0;
else i=2;

diff -urN -X dontdiff linux/drivers/net/pcmcia/wavelan_cs.c rb/drivers/net/pcmcia/wavelan_cs.c
@@ -1774,7 +1774,7 @@
 #if WIRELESS_EXT > 7
   const int	BAND_NUM = 10;	/* Number of bands */
   int		c = 0;		/* Channel number */
-#endif WIRELESS_EXT
+#endif /* WIRELESS_EXT */
 
   /* Read the frequency table */
   fee_read(base, 0x71 /* frequency table */,

@@ -1792,7 +1792,7 @@
 	      (c < BAND_NUM))
 	  c++;
 	list[i].i = c;	/* Set the list index */
-#endif WIRELESS_EXT
+#endif /* WIRELESS_EXT */
 
 	/* put in the list */
 	list[i].m = (((freq + 24) * 5) + 24000L) * 10000;

diff -urN -X dontdiff linux/drivers/net/pcmcia/xircom_tulip_cb.c rb/drivers/net/pcmcia/xircom_tulip _cb.c

@@ -2388,7 +2388,7 @@
 #ifdef CARDBUS
 		if (tp->chip_id == X3201_3)
 			tp->tx_aligned_skbuff[i] = dev_alloc_skb(PKT_BUF_SZ);
-#endif CARDBUS
+#endif /* CARDBUS */
 	}
 	tp->tx_ring[i-1].buffer2 = virt_to_bus(&tp->tx_ring[0]);
 }

diff -urN -X dontdiff linux/drivers/net/pppoe.c rb/drivers/net/pppoe.c
@@ -737,7 +737,7 @@
 		err = 0;
 		break;
 
-	default:
+	default:;
 	};
 
 	return err;

diff -urN -X dontdiff linux/drivers/net/tokenring/ibmtr.c rb/drivers/net/tokenring/ibmtr.c
@@ -1181,7 +1181,7 @@
 				isa_writeb(~CMD_IN_SRB, ti->mmio + ACA_OFFSET + ACA_RESET + ISRA_ODD);
 				isa_writeb(~SRB_RESP_INT, ti->mmio + ACA_OFFSET + ACA_RESET + ISRP_ODD);
 
-			  skip_reset:
+			  skip_reset:;
 			} /* SRB response */
 
 			if (status & ASB_FREE_INT) { /* ASB response */

diff -urN -X dontdiff linux/drivers/net/wan/lmc/lmc.h rb/drivers/net/wan/lmc/lmc.h
@@ -29,4 +29,5 @@
 static void lmcEventLog( u_int32_t EventNum, u_int32_t arg2, u_int32_t arg3 );
 #endif
 
-#endif
\ No newline at end of file
+#endif
+

diff -urN -X dontdiff linux/drivers/net/wan/lmc/lmc_proto.h rb/drivers/net/wan/lmc/lmc_proto.h
@@ -12,4 +12,5 @@
 void lmc_proto_netif(lmc_softc_t *sc, struct sk_buff *skb);
 int lmc_skb_rawpackets(char *buf, char **start, off_t offset, int len, int unused);
 
-#endif
\ No newline at end of file
+#endif
+

diff -urN -X dontdiff linux/drivers/scsi/NCR5380.h rb/drivers/scsi/NCR5380.h
@@ -373,6 +373,6 @@
 }
 #endif /* defined(i386) || defined(__alpha__) */
 #endif /* defined(REAL_DMA)  */
-#endif __KERNEL_
+#endif /* __KERNEL_ */
 #endif /* ndef ASM */
 #endif /* NCR5380_H */

diff -urN -X dontdiff linux/drivers/scsi/NCR53c406a.c rb/drivers/scsi/NCR53c406a.c
@@ -221,7 +221,7 @@
     (void *)0xc8000
 };
 #define ADDRESS_COUNT (sizeof( addresses ) / sizeof( unsigned ))
-#endif USE_BIOS
+#endif /* USE_BIOS */
 		       
 /* possible i/o port addresses */
 static unsigned short ports[] =

@@ -244,7 +244,7 @@
     { "Copyright (C) Acculogic, Inc.\r\n2.8M Diskette Extension Bios ver 4.04.03 03/01/1993", 61, 

82 },
};
#define SIGNATURE_COUNT (sizeof( signatures ) / sizeof( struct signature ))
-#endif USE_BIOS
+#endif /* USE_BIOS */

/* ============================================================ */

@@ -347,7 +347,7 @@

     return tmp;

}
-#endif USE_DMA
+#endif /* USE_DMA */

#if USE_PIO
static __inline__ int NCR53c406a_pio_read(unsigned char *request,
@@ -455,7 +455,7 @@

     }
     return 0;

}
-#endif USE_PIO
+#endif /* USE_PIO */

int __init
NCR53c406a_detect(Scsi_Host_Template * tpnt){
@@ -481,7 +481,7 @@

     }
     
     DEB(printk("NCR53c406a BIOS found at %X\n", (unsigned int) bios_base););

-#endif USE_BIOS
+#endif /* USE_BIOS */

 #ifdef PORT_BASE
     if (check_region(port_base, 0x10)) /* ports already snatched */
@@ -510,7 +510,7 @@
             }
         }
     }

-#endif PORT_BASE
+#endif /* PORT_BASE */

     if(!port_base){		/* no ports found */
         printk("NCR53c406a: no available ports found\n");
@@ -564,7 +564,7 @@
     }
     
     DEB(printk("Allocated DMA channel %d\n", dma_chan));

-#endif USE_DMA
+#endif /* USE_DMA */

     tpnt->present = 1;
     tpnt->proc_name = "NCR53c406a";
@@ -804,8 +804,8 @@
     printk("\n");
 #else
     printk(", pio=%02x\n", pio_status);

-#endif USE_DMA
-#endif NCR53C406A_DEBUG
+#endif /* USE_DMA */
+#endif /* NCR53C406A_DEBUG */

     if(int_reg & 0x80){         /* SCSI reset intr */
         rtrc(3);
@@ -824,7 +824,7 @@
         current_SC->scsi_done(current_SC);
         return;
     }

-#endif USE_PIO
+#endif /* USE_PIO */

     if(status & 0x20) {		/* Parity error */
         printk("NCR53c406a: Warning: parity error!\n");
@@ -869,7 +869,7 @@
 #if USE_DMA			/* No s/g support for DMA */
             NCR53c406a_dma_write(current_SC->request_buffer, 
                                  current_SC->request_bufflen);
-#endif USE_DMA
+#endif /* USE_DMA */
             outb(TRANSFER_INFO | DMA_OP, CMD_REG); 
 #if USE_PIO
             if (!current_SC->use_sg) /* Don't use scatter-gather */
@@ -884,7 +884,7 @@
                 }
             }
             REG0;
-#endif USE_PIO
+#endif /* USE_PIO */
         }
         break;

@@ -898,7 +898,7 @@

 #if USE_DMA			/* No s/g support for DMA */
             NCR53c406a_dma_read(current_SC->request_buffer, 
                                 current_SC->request_bufflen);
-#endif USE_DMA
+#endif /* USE_DMA */
             outb(TRANSFER_INFO | DMA_OP, CMD_REG); 
 #if USE_PIO
             if (!current_SC->use_sg) /* Don't use scatter-gather */
@@ -913,7 +913,7 @@
                 }
             }
             REG0;
-#endif USE_PIO
+#endif /* USE_PIO */
         }
         break;

@@ -995,7 +995,7 @@

     return irq;

}
-#endif IRQ_LEV
+#endif /* IRQ_LEV */

static void chip_init()
{

diff -urN -X dontdiff linux/drivers/scsi/aic7xxx.c rb/drivers/scsi/aic7xxx.c
@@ -10099,7 +10099,7 @@
       } /* while(pdev=....) */
     } /* for PCI_DEVICES */
   } /* PCI BIOS present */
-#endif CONFIG_PCI
+#endif /* CONFIG_PCI */
 
 #if defined(__i386__) || defined(__alpha__)
   /*

diff -urN -X dontdiff linux/drivers/scsi/osst.c rb/drivers/scsi/osst.c
@@ -3668,7 +3668,7 @@
 #if DEBUG
 		if (debugging)
 		    printk(OSST_DEB_MSG "osst%d: Locking drive door.\n", dev);
-#endif;
+#endif
 		break;
 
 	 case MTUNLOCK:

@@ -3678,7 +3678,7 @@
 #if DEBUG
 		if (debugging)
 		   printk(OSST_DEB_MSG "osst%d: Unlocking drive door.\n", dev);
-#endif;
+#endif
 	break;
 
 	 case MTSETBLK:           /* Set block length */

diff -urN -X dontdiff linux/drivers/scsi/scsi.c rb/drivers/scsi/scsi.c
@@ -2355,7 +2355,7 @@
 	case MODULE_SCSI_CONST:
 	case MODULE_SCSI_IOCTL:
 		break;
-	default:
+	default:;
 	}
 	return;
 }

diff -urN -X dontdiff linux/fs/isofs/inode.c rb/fs/isofs/inode.c
@@ -616,7 +616,7 @@
 #ifndef IGNORE_WRONG_MULTI_VOLUME_SPECS
 	  if (isonum_723 (h_pri->volume_set_size) != 1)
 		goto out_no_support;
-#endif IGNORE_WRONG_MULTI_VOLUME_SPECS
+#endif /* IGNORE_WRONG_MULTI_VOLUME_SPECS */
 	  s->u.isofs_sb.s_nzones = isonum_733 (h_pri->volume_space_size);
 	  s->u.isofs_sb.s_log_zone_size = isonum_723 (h_pri->logical_block_size);
 	  s->u.isofs_sb.s_max_size = isonum_733(h_pri->volume_space_size);

@@ -625,7 +625,7 @@
 #ifndef IGNORE_WRONG_MULTI_VOLUME_SPECS
 	  if (isonum_723 (pri->volume_set_size) != 1)
 		goto out_no_support;
-#endif IGNORE_WRONG_MULTI_VOLUME_SPECS
+#endif /* IGNORE_WRONG_MULTI_VOLUME_SPECS */
 	  s->u.isofs_sb.s_nzones = isonum_733 (pri->volume_space_size);
 	  s->u.isofs_sb.s_log_zone_size = isonum_723 (pri->logical_block_size);
 	  s->u.isofs_sb.s_max_size = isonum_733(pri->volume_space_size);

diff -urN -X dontdiff linux/include/linux/if_frad.h rb/include/linux/if_frad.h
@@ -194,7 +194,7 @@
 
 int (*dlci_ioctl_hook)(unsigned int, void *);
 
-#endif __KERNEL__
+#endif /* __KERNEL__ */
 
 #endif /* CONFIG_DLCI || CONFIG_DLCI_MODULE */
 

diff -urN -X dontdiff linux/kernel/sched.c rb/kernel/sched.c
@@ -548,7 +548,7 @@
 			}
 		default:
 			del_from_runqueue(prev);
-		case TASK_RUNNING:
+		case TASK_RUNNING:;
 	}
 	prev->need_resched = 0;
 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

[...]

	grep -q '^#endif [A-Za-z0-9_]' $i 2>/dev/null
	if [ $? == 0 ]; then
		mv $i $i~
		sed 's/^#endif \([A-Za-z0-9_]\+$\)/#endif \/\* \1 \*\//'<$i~>$i
	fi

#endif can have white space before '#' and between '#' and 'endif', it can have tabs instead of spaces, the spurious text is anything that does not start with '/'. Time to start a new one liner contest ;) ...

find -type f -name '*.[chS]' | xargs perl -lpi -e 's:^(\s*#\s*endif)\s+([^/\s].*)$:\1\t/* \2 */:;'

It even preserves the existing #endif layout. That regexp does not catch #endif /foo, it assumes that '/' always starts a comment. The extra complexity to catch that rare case is not worth it. The command changed 97 files on base 2.4.0.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It's not valid in current ISO C. So gcc warns about it

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It still compiles and works just as well as it does without the patch.
Without the patch it says:
warning: deprecated use of label at end of compound statement

My patches are basically telling the compiler to be quiet. If you use egcs it won't give these warnings even if the code hasn't been patched.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

No, it is not a common idiom in C. It has _never_ been valid C.

GCC originally allowed it due to a mistake in the grammar; we now warn for it. Fix your source.

r~
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

[about labels w/o statements after them]

Since neither -ansi nor -std=foo was specified, gcc should just shut up and be happy. Consider this as another GNU extension.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

It has to do with ; "a label at the end of a compound statement..." This has never been correctly allowed. Many don't realilize that

	case 'X':
	case 'Y':
	default:

... are all labels. Modern compilers are now enforcing the rules. When a 'switch' is a compound statement, tt follows the rules for other compound statements. For instance, you can code (correctly)

	switch(a) case 1: a--;

... this, with no braces at all. If a == 1, it gets changed to 0, otherwise it is untouched. If we need another test, it becomes a compound statement requiring braces as:

	switch(a) { case 1: a--; default: }

Observe that we have tricked the compiler into generating code without using a ';' denoting the end of a statement. The standards makers don't like this and and now requiring that the above be coded as:

	switch(a) { case 1: a--; default: ; }
	                                  ^___ no tricks allowed.

A 'program unit', denoted by {} braces has never required a terminating semicolon, so putting a ';' at the end of the physical statement just won't do it in this case.

Cheers,
Dick Johnson

Penguin : Linux version 2.4.0 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of course you get it all back when you reboot..."; Actual explanation obtained from the Micro$oft help desk.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

No, it was a gcc bug that gcc accepted the syntax in the first place.

Let the gcc people fix the bugs they find without complaining about them. After all, gcc would have been perfectly correct in signalling this as a syntax error, and aborted compilation. The fact that gcc only warns about it is a sign of grace - it's not as if it is a _useful_ extension that gives the programmer anything new and should be left in for that reason.

We'll fix up the kernel. That's where the bug is.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Sure. I'd always thought it was intentional though.

It is slightly useful: appearance, ease-of-use, etc.

Code is partly for humans to read, and is often written by humans too. The standard syntax has worthless ugly junk. The extension doesn't break any valid K&R, C90, or C99 code.

Obviously, considering the kernel source, people _like_ this:

void foo(void){
if(bar()) goto out;
baz();
out:
}

If it broke valid C99 code, then sure, it would have to go.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I do agree that the gcc extension is more readable, and I have to admit that I was a bit surprised that it wasn't legal - I guess I've just been using gcc for too long.

But at the same time, I do agree with the gcc maintainers that maintaining extensions is only worth it if those extensions really buy you something, and if not, you're better off trying to force people to follow documented standards. The discussion about 'rmdir(".")' that has been going on on linux-kernel is very much the same thing - even if 'rmdir(".")' might be slightly useful for some case, it's not useful enough to warrant extra code and complexity.

You may argue that right now the new gcc warning _is_ extra code and complexity, and that it would actually have been easier for gcc maintainers to just continue with the old parser. You'd be right on a small scale. You'd be wrong in the long run - if gcc ever wants to rewrite the parser in the future, it's going to be much easier for them if they don't have to worry about undocumented extensions etc.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Infact I don't blame gcc maintainers for that, but the standard. Ok, minor issue.

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Yeah, and nothing we can do about it any more.. Oh, well.

The fact is that the

	case xxx: ;

syntax is fairly ugly, so I'd prefer the fixup patches to look more like

	case xxx:
		/* fallthrough */ ;
	}

or something (or maybe just a "break" statement), just so that we don't turn the poor C language into line noise (can anybody say "perl" ;)

I have to say, I think it was Pascal had this "no semicolon needed before an 'end'" rule, and I always really hated that. The C statement rules make a lo tmore sense, and requiring a statement after a case statement is probably a very good requirement from a language standpoint. It's just not very pretty - but adding a break or a comment will at least separate out the colon and the semi-colon a bit.

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Of course, you don't mean that the fallthrough comment and the break statement have the same functionality! (well you put the closing bracket and I agree that for the last case it's the same).

I think it's always a good idea to put a comment when you relay on the (otherwise implicit) fallthrough, especially if there are statements in between:

	case xxx:
		stm1;
		/* fallthrough */
	case yyy:
		stm2;

but, note the absence of the semicolon after the comment (which is legal, if stm1 is present).

The above is just what i feel "natural": it says that case xxx is almost the same as case yyy but for a single (initial) statement. It looks like "good C" to my eyes.

But what happens if I delete the stm1 line? We have:

	case xxx:
		/* fallthrough */
	case yyy:
		stm2;

which is wrong. It could be:

	case xxx:
	case yyy:
		stm2;

which says that xxx and yyy are just the same, and looks fine to me. Here the "fallthrough" rule is very readable. Too bad it's wrong.

So let me state the rule:
1) always put a comment when relaying on the fallthrough, followed by a semicolon, even if not strictly necessary (so that you can delete the statements above it later).
2) put the same comment or a break after the last case.

Maybe it's time to update Documentation/CodingStyle? (a very pleasant reading, BTW. I must say I sometimes re-read it just for fun)

Well, I've always hated the Pascal rule, too. It makes you feel that there's a missing semicolon AND it forces you to add one when you decide to add new statements after the last one (i used to forget it most of the time). Now, this C rule makes you see a semicolon where you don't expect it to be, after a comment. I'm used to see comments after the semicolon, as in:

	int	foo;	/* this is foo */

or alone. The only place I'd expect to see a comment before a semicolon is in:

	for(;;)
		/* nop */;

but I'd write that differently anyway:

	for(;;) {
		continue;
	}

which makes easier to add statements inside the loop. For maximum readability I'd leave the "continue" even if there are other statements:

	for(;;) {
		stm1;
		if (condition) {
			break;
		}
		stm2;
		continue;
	}

(it's like a comment that says "loop again", opposed to the break which says "stop looping") but I agree that's overkill. B-)

.TM.

      ____/  ____/   /
     /      /       /			Marco Colombo
    ___/  ___  /   /		      Technical Manager
   /          /   /			 ESI s.r.l.
 _____/ _____/  _/		       [email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

AFAIK, that's perfectly correct. It's only the case where you have a label at the end of a block (without a statement following it) where it's an error.

In the grammar, a statement must follow a label, but a labeled-statement is a type of statement, so you can stack labels as much as you want, as long as there's a statement somewhere after them.

That is, assuming I'm reading the standard right (ISO/IEC 9899:1990, Section 6.6, 6.6.1).

Alan Shutko <[email protected]> - In a variety of flavors!
Programmers do it bit by bit.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Opps, sorry, I misunderstood Linus' message, then.

.TM.

      ____/  ____/   /
     /      /       /			Marco Colombo
    ___/  ___  /   /		      Technical Manager
   /          /   /			 ESI s.r.l.
 _____/ _____/  _/		       [email protected]


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Note that the warning case we're discussing was really only about case statements at the end of a compound statement.

In the middle of compound statements we're already fine: it's only the corner case of a case "statement" without the statement that gcc historically used to accept without warning, and that the gcc people only recently noticed that they really shouldn't accept at all.

So that's why a comment and a "break" is equivalent. ONLY for the special case of the new compile warning, though, obviously (see the subject line, but yes, I should have made that more explicit).

Linus

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I see. The choice between the comment and the 'break' can be left to the author, maybe he knows what is the best one (the one that will let us add a new case with less effort).

If you bother setting a styleguide rule, I believe the latter is more common.

But still I dislike semicolons *after* comments. Why not:

	case a:
		...
		; /* fallthrough */
	case b:
		...
		; /* fallthrough */
	}

It looks just a little less ugly to my eyes (please take a few seconds to get used at it), maybe it's because it's reminiscent of shell double-semicolon. In general, I find it more readable when comments are somewhat "outside" the C code.

[ Note: I still put it after the first case, both for documenting the fallthrough behaviour, which is a good thing anyway, and to allow deleting the last case with no thinking of it. ]

I'd even put a tab before the comment.

	case b:
		...
		;	/* fallthrough */
	}

Ok, that's really a matter of taste. You spend more time in front of the source than me, and you know better. I'd like you to set a kind of rule, because it's something that many are not used to see and it's easier to recognise it if it looks the same everywhere.

.TM.

      ____/  ____/   /
     /      /       /			Marco Colombo
    ___/  ___  /   /		      Technical Manager
   /          /   /			 ESI s.r.l.
 _____/ _____/  _/		       [email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

I'll redo my patch to use break; and then I'll resend it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Me too ;)

Andrea
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

gcc

C.

we

extension.

place.

about them.

as a

warns about

reason.

kernel" in

Here's a patch that takes care of all warnings of this type I have found in 2.4.0. If anyone else finds more of these warnings let me know or look at me patch to see how to do it yourself.

diff -urN -X dontdiff linux/drivers/block/cpqarray.c rb/drivers/block/cpqarray.c
@@ -1321,7 +1321,7 @@
 	case IDA_WRITE_MEDIA:
 		kfree(p);
 		break;
-	default:
+	default:;
 		/* Nothing to do */
 	}
 

diff -urN -X dontdiff linux/drivers/cdrom/cm206.c rb/drivers/cdrom/cm206.c
@@ -1283,7 +1283,7 @@
   case 1: 
     kfree(cd);
     release_region(cm206_base, 16);
-  default:
+  default:;
   }
 }
 

diff -urN -X dontdiff linux/drivers/cdrom/mcd.c rb/drivers/cdrom/mcd.c
@@ -1158,7 +1158,7 @@
       return;
     }
     blk_cleanup_queue(BLK_DEFAULT_QUEUE(MAJOR_NR));
-  default:
+  default:;
   }
 }
 

diff -urN -X dontdiff linux/drivers/char/ftape/lowlevel/fdc-isr.c rb/drivers/char/ftape/lowlevel/fdc-isr.c
@@ -81,7 +81,7 @@
 	case overrun_error:
 		TRACE(ft_t_noise, "overrun error");
 		break;
-	default:
+	default:;
 	}
 	TRACE_EXIT;
 }

@@ -184,7 +184,7 @@
 	case no_data_error:
 		ft_history.no_data_errors++;
 		break;
-	default:
+	default:;
 	}
 }
 

diff -urN -X dontdiff linux/drivers/ieee1394/pcilynx.c rb/drivers/ieee1394/pcilynx.c
@@ -1485,7 +1485,7 @@
                 pci_free_consistent(lynx->dev, LOCALRAM_SIZE, lynx->pcl_mem,
                                     lynx->pcl_mem_dma);
 #endif
-        case clear:
+        case clear:;
                 /* do nothing - already freed */
         }
 

diff -urN -X dontdiff linux/drivers/isdn/eicon/eicon_idi.c rb/drivers/isdn/eicon/eicon_idi.c
@@ -2506,7 +2506,7 @@
 						case ISDN_PROTO_L2_TRANS:
 							idi_do_req(ccard, chan, N_CONNECT, 1);
 							break;
-						default:
+						default:;
 							/* On most incoming calls we use automatic connect */
 							/* idi_do_req(ccard, chan, N_CONNECT, 1); */
 					}

diff -urN -X dontdiff linux/drivers/isdn/eicon/eicon_pci.c rb/drivers/isdn/eicon/eicon_pci.c
@@ -86,7 +86,7 @@
 				printk(KERN_INFO "%s: DriverID='%s' CardID=%d\n",
 					eicon_ctype_name[ctype], did, card_id);
 			}
-err:
+err:;
 		}
 		pCard++;
 	}

diff -urN -X dontdiff linux/drivers/isdn/hisax/isac.c rb/drivers/isdn/hisax/isac.c
@@ -445,7 +445,7 @@
 				if (cs->debug & L1_DEB_MONITOR)
 					debugl1(cs, "ISAC %02x -> MOX1", cs->dc.isac.mon_tx[cs->dc.isac.mon_txp -1]);
 			}
-		      AfterMOX1:
+		      AfterMOX1:;
 #endif
 		}
 	}

diff -urN -X dontdiff linux/drivers/isdn/isdn_tty.c rb/drivers/isdn/isdn_tty.c
@@ -3773,7 +3773,7 @@
                                                 sprintf(ds, "\r\n%d", info->emu.charge);
                                                 isdn_tty_at_cout(ds, info);
                                                 break;
-					default:
+					default:;
 				}
 				break;
 #ifdef DUMMY_HAYES_AT

diff -urN -X dontdiff linux/drivers/isdn/isdn_v110.c rb/drivers/isdn/isdn_v110.c
@@ -600,7 +600,7 @@
 					case ISDN_PROTO_L2_V11038:
 						dev->v110[idx] = isdn_v110_open(V110_38400, hdrlen, maxsize);
 						break;
-					default:
+					default:;
 				}
 				if ((v = dev->v110[idx])) {
 					while (v->SyncInit) {

diff -urN -X dontdiff linux/drivers/md/md.c rb/drivers/md/md.c
@@ -2588,7 +2588,7 @@
 			err = md_put_user (read_ahead[
 				MAJOR(dev)], (long *) arg);
 			goto done;
-		default:
+		default:;
 	}
 
 	/*

@@ -2607,7 +2607,7 @@
 				err = -EEXIST;
 				goto abort;
 			}
-		default:
+		default:;
 	}
 	switch (cmd)
 	{

@@ -2660,7 +2660,7 @@
 			}
 			goto done;
 
-		default:
+		default:;
 	}
 
 	/*

diff -urN -X dontdiff linux/drivers/media/video/msp3400.c rb/drivers/media/video/msp3400.c
@@ -1499,7 +1499,7 @@
 			 (int)msp3400c_read(client, I2C_MSP3400C_DFP, 0x1c));
 		break;
 #endif
-	default:
+	default:;
 		/* nothing */
 	}
 	return 0;

diff -urN -X dontdiff linux/drivers/mtd/nftlmount.c rb/drivers/mtd/nftlmount.c
@@ -118,7 +118,7 @@
 				break;
 			}
 		}
-	ReplUnitTable:
+	ReplUnitTable:;
 	}
 
 	if (boot_record_count == 0) {

@@ -652,7 +652,7 @@
 				}
 			}
 		}
-	examine_ReplUnitTable:
+	examine_ReplUnitTable:;
 	}
 
 	/* second pass to format unreferenced blocks  and init free block count */

diff -urN -X dontdiff linux/net/802/cl2llc.c rb/net/802/cl2llc.c
@@ -96,7 +96,7 @@
 				else
 					llc_interpret_pseudo_code(lp, REJECT1, skb, NO_FRAME);
 				break;
-			default:
+			default:;
 		}
 		if(lp->llc_callbacks)
 		{

@@ -497,7 +497,7 @@
 				else
 					lp->f_flag = fr->i_hdr.i_pflag;
 				break;
-			default:
+			default:;
 		}
 		pc++;	
 	}

diff -urN -X dontdiff linux/net/802/llc_macinit.c rb/net/802/llc_macinit.c
@@ -125,7 +125,7 @@
 				free=0;
 				break;
 
-			default:
+			default:;
 				/*
 				 *	All other type 1 pdus ignored for now
 				 */

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/