drivers/infiniband/mlx/mad.c misplaced ;

drivers/infiniband/hw/mlx4/mad.c int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int ignore_mkey, int ignore_bkey, · line 112

Quoted 12 lines · Dave Jones

Signed-off-by Dave Jones <[email protected]>

drivers/infiniband/hw/mlx4/mad.c +1 -1

index 3330917..0ed02b7 100644
@@ -109,7 +109,7 @@ int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int ignore_mkey, int ignore_bkey,
 			   in_modifier, op_modifier,
 			   MLX4_CMD_MAD_IFC, MLX4_CMD_TIME_CLASS_C);
 
-	if (!err);
+	if (!err)

There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * arch/sh/boards/se/7343/io.c:    if (0) ;
drivers/atm/iphase.c:     if (!desc1) ;
drivers/infiniband/hw/mlx4/mad.c:       if (!err);
drivers/isdn/capi/capiutil.c:   else if (c <= 0x0f);
drivers/net/tokenring/ibmtr.c:  else if (ti->shared_ram_paging == 0xf);  /* No paging in adapter */
drivers/s390/scsi/zfcp_erp.c:           if (status == ZFCP_ERP_SUCCEEDED) ;     /* no further action */
fs/hostfs/hostfs_user.c:        if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
net/netfilter/xt_u32.c:         if (skb->len < 4 || pos > skb->len - 4);
sound/pci/au88x0/au88x0_synth.c:                                if (eax == 0) ;

Quoted 12 lines · Joe Perches

On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote:
There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * 
arch/sh/boards/se/7343/io.c:    if (0) ;
drivers/atm/iphase.c:     if (!desc1) ;
drivers/infiniband/hw/mlx4/mad.c:       if (!err);
drivers/isdn/capi/capiutil.c:   else if (c <= 0x0f);
drivers/net/tokenring/ibmtr.c:  else if (ti->shared_ram_paging == 0xf);  /* No paging in adapter */
drivers/s390/scsi/zfcp_erp.c:           if (status == ZFCP_ERP_SUCCEEDED) ;     /* no further action */
fs/hostfs/hostfs_user.c:        if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
net/netfilter/xt_u32.c:         if (skb->len < 4 || pos > skb->len - 4);
sound/pci/au88x0/au88x0_synth.c:                                if (eax == 0) ;

sounds like an excellent candidate check for checkpatch.pl !!!

Quoted 12 lines · Auke

Quoted 11 lines · Joe Perches

On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote:
There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * arch/sh/boards/se/7343/io.c:    if (0) ;
drivers/atm/iphase.c:     if (!desc1) ;
drivers/infiniband/hw/mlx4/mad.c:       if (!err);
drivers/isdn/capi/capiutil.c:   else if (c <= 0x0f);
drivers/net/tokenring/ibmtr.c:  else if (ti->shared_ram_paging == 0xf);  /* No paging in adapter */
drivers/s390/scsi/zfcp_erp.c:           if (status == ZFCP_ERP_SUCCEEDED) ;     /* no further action */
fs/hostfs/hostfs_user.c:        if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
net/netfilter/xt_u32.c:         if (skb->len < 4 || pos > skb->len - 4);
sound/pci/au88x0/au88x0_synth.c:                                if (eax == 0) ;

Quoted 1 line

sounds like an excellent candidate check for checkpatch.pl !!!

I think it's poor style and all of these should go away.

the netfilter one appears to be a real error too.

Quoted 18 lines · Joe Perches

Quoted 8 lines · Dave Jones

Signed-off-by Dave Jones <[email protected]>

drivers/infiniband/hw/mlx4/mad.c +0 -0

index 3330917..0ed02b7 100644

drivers/infiniband/hw/mlx4/mad.c int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int · line 113

Quoted 6 lines

drivers/infiniband/hw/mlx4/mad.c +1 -1

@@ -109,7 +109,7 @@ int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int
 ignore_mkey, int ignore_bkey,
                in_modifier, op_modifier,
                MLX4_CMD_MAD_IFC, MLX4_CMD_TIME_CLASS_C);
 
-    if (!err);
+    if (!err)

There's more than a few of these (not inspected).

Quoted 13 lines

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" *
arch/sh/boards/se/7343/io.c:    if (0) ;
drivers/atm/iphase.c:     if (!desc1) ;
drivers/infiniband/hw/mlx4/mad.c:       if (!err);
drivers/isdn/capi/capiutil.c:   else if (c <= 0x0f);
drivers/net/tokenring/ibmtr.c:  else if (ti->shared_ram_paging ==
0xf);  /* No paging in adapter */
drivers/s390/scsi/zfcp_erp.c:           if (status ==
ZFCP_ERP_SUCCEEDED) ;     /* no further action */
fs/hostfs/hostfs_user.c:        if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
net/netfilter/xt_u32.c:         if (skb->len < 4 || pos > skb->len - 4);
sound/pci/au88x0/au88x0_synth.c:                                if
(eax == 0) ;

sounds like an excellent candidate check for checkpatch.pl !!!

drivers/infiniband/hw/mlx4/mad.c int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int ignore_mkey, int ignore_bkey, · line 112

Quoted 12 lines · Dave Jones

Signed-off-by Dave Jones <[email protected]>

drivers/infiniband/hw/mlx4/mad.c +1 -1

index 3330917..0ed02b7 100644
@@ -109,7 +109,7 @@ int mlx4_MAD_IFC(struct mlx4_ib_dev *dev, int ignore_mkey, int ignore_bkey,
 			   in_modifier, op_modifier,
 			   MLX4_CMD_MAD_IFC, MLX4_CMD_TIME_CLASS_C);
 
-	if (!err);
+	if (!err)

There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * 

drivers/s390/scsi/zfcp_erp.c: if (status == ZFCP_ERP_SUCCEEDED) ; /* no further action */

Quoted 4 lines

There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * 

...I guess those guys hunting for broken busyloops in the other thread could also benefit from similar searching commands introduced in this thread... ...Ccing Satyam to caught their attention too.

./drivers/isdn/hisax/hfc_pci.c
125: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
155: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
1483: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
--
./drivers/isdn/hisax/hfc_sx.c
377: if (Read_hfc(cs, HFCSX_INT_S1)) ;
407: if (Read_hfc(cs, HFCSX_INT_S2)) ;
1246: if (Read_hfc(cs, HFCSX_INT_S1)) ;
--

Quoted 6 lines · Ilpo Järvinen

...I guess those guys hunting for broken busyloops in the other thread could also benefit from similar searching commands introduced in this thread... ...Ccing Satyam to caught their attention too.

Quoted 10 lines

./drivers/isdn/hisax/hfc_pci.c
125: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
155: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
1483: if (Read_hfc(cs, HFCPCI_INT_S1)) ;
--
./drivers/isdn/hisax/hfc_sx.c
377: if (Read_hfc(cs, HFCSX_INT_S1)) ;
407: if (Read_hfc(cs, HFCSX_INT_S2)) ;
1246: if (Read_hfc(cs, HFCSX_INT_S1)) ;
--

These are workaround to not get compiler warnings about ignored return values I got some time ago under some architecture.

...I guess those guys hunting for broken busyloops in the other thread could also benefit from similar searching commands introduced in this thread... ...Ccing Satyam to caught their attention too.

Quoted 5 lines · Joe Perches

Quoted 4 lines

There's more than a few of these (not inspected).

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * 

...Hmm, I plugged in "a preprocessor" too to manage with non compliant coding styles :-). Please understand that the line numbers are not an exact match due to preprocessor changes:

$ for i in `find . -name '*.[ch]'`; do echo $i; indent -npro -kr -i8 -ts8 
-sob -l8000 -ss -ncs -cp1 -nhnl -st $i | egrep -n "[[:space:]]if [(].*[)] ;$";
done | grep -B1 "^[^.]"

fs/hostfs/hostfs_user.c: if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;

Quoted 1 line · Joe Perches

fs/hostfs/hostfs_user.c: if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;

This one can be deleted, but I think I did it for documentation reasons, to make it clear that ctime handling wasn't left out by mistake.

We normally use "comments" for that, not dead code that a compiler then elids ;-)

[PATCH] hostfs: Remove pointless if statement

And replace with comment saying we don't handle ctime. Also some codingstyle while I was there.

Seems that a trailing ';' has slipped onto the end of the access_ok check here, such that we will always return -EFAULT.

It seems we have gained an extraneous trailing ';' on one of the wait loops in scif_sercon_putc(). Although this is completely benign as the apparent payload is also the empty statement, it invites error in the future. Clean it up now.

Signed-off-by Andy Whitcroft <[email protected]>

It seems a trailing ';' has slipped into mlx4_MAD_IFC() which causes the response to be copied out unconditionally.

It seems an extraneous trailing ';' has slipped into the skb length checks in u32_match_it() triggering an unconditional missmatch.

A couple of people suggested adding checks to checkpatch for trailing semicolons on conditionals, where the conditional block may not be actually conditional:

	if (err);
		return err;

While regression testing the changes, I ran these checks across the whole of 2.6.23-rc3 and there appear to be 5 places where this is occurs (above and beyond the IPv6 one which triggered this effort) and a benign use which could be confused later which it seems safest to fix.

Following this email are 6 patches for these issues, relevant maintainers cc'd. All against 2.6.23-rc3

$ egrep -r --include=*.c "\bif[[:space:]]*\([^\)]*\)[[:space:]]*\;" * 
arch/sh/boards/se/7343/io.c:    if (0) ;
drivers/atm/iphase.c:     if (!desc1) ;
drivers/infiniband/hw/mlx4/mad.c:       if (!err);
drivers/isdn/capi/capiutil.c:   else if (c <= 0x0f);
drivers/net/tokenring/ibmtr.c:  else if (ti->shared_ram_paging == 0xf);  /* No paging in adapter */
drivers/s390/scsi/zfcp_erp.c:           if (status == ZFCP_ERP_SUCCEEDED) ;     /* no further action */
fs/hostfs/hostfs_user.c:        if(attrs->ia_valid & HOSTFS_ATTR_CTIME) ;
net/netfilter/xt_u32.c:         if (skb->len < 4 || pos > skb->len - 4);
sound/pci/au88x0/au88x0_synth.c:                                if (eax == 0) ;

A couple of people suggested adding checks to checkpatch for trailing semicolons on conditionals, where the conditional block may not be actually conditional:

	if (err);
		return err;

While regression testing the changes, I ran these checks across the whole of 2.6.23-rc3 and there appear to be 5 places where this is occurs (above and beyond the IPv6 one which triggered this effort) and a benign use which could be confused later which it seems safest to fix.

Following this email are 6 patches for these issues, relevant maintainers cc'd. All against 2.6.23-rc3