Re: drivers/infiniband/mlx/mad.c misplaced ;
On 2007/08/16 , at 13:01, Karsten Keil wrote: On Thu, Aug 16, 2007 at 01:22:04PM +0300, Ilpo Järvinen wrote: ...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)) ; -- These are workaround to not get compiler warnings about ignored return values I got some time ago under some architecture. Maybe '(void) Read_hfc(cs, HFCSX_INT_S1)' is a better option to get rid of the warnings. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
On Wed, Aug 15, 2007 at 05:40:11PM -0700, Joe Perches wrote: > $ 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) > ; On Thu, Aug 16, 2007 at 02:18:40PM +0100, Andy Whitcroft wrote: > 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 > It looks like you may want to refine your search parameters to match the above, as there are at least a few cases where the space exists. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
On Wed, Aug 15, 2007 at 05:40:11PM -0700, Joe Perches wrote: > 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. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
Kok, Auke wrote: > Joe Perches wrote: >> On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote: >>> Signed-off-by: Dave Jones <[EMAIL PROTECTED]> >>> >>> diff --git a/drivers/infiniband/hw/mlx4/mad.c >>> b/drivers/infiniband/hw/mlx4/mad.c >>> index 3330917..0ed02b7 100644 >>> --- a/drivers/infiniband/hw/mlx4/mad.c >>> +++ b/drivers/infiniband/hw/mlx4/mad.c >>> @@ -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) ; > > sounds like an excellent candidate check for checkpatch.pl !!! Yep. My scan of 2.6.23-rc3 with a checkpatch with this new test added, found 6 which seemed wrong. -apw - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
Hi Ilpo, On Thu, 16 Aug 2007, Ilpo Järvinen wrote: > > ...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. > > > > On Wed, Aug 15, 2007 at 05:40:11PM -0700, Joe Perches wrote: > > > > > > 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 "^[^.]" Thanks, looks useful, will check with this. Satyam
Re: drivers/infiniband/mlx/mad.c misplaced ;
On Thu, Aug 16, 2007 at 01:22:04PM +0300, Ilpo Järvinen wrote: > > ...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)) ; > -- These are workaround to not get compiler warnings about ignored return values I got some time ago under some architecture. -- Karsten Keil SuSE Labs ISDN and VOIP development SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg) - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
...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. > On Wed, Aug 15, 2007 at 05:40:11PM -0700, Joe Perches wrote: > > > > 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 "^[^.]" ./arch/arm/mach-omap1/leds-innovator.c 97: if (led_state & LED_STATE_ENABLED) ; -- ./arch/mips/sibyte/cfe/console.c 23: if (written < 0) ; 32: if (written < 0) ; -- ./arch/powerpc/kernel/legacy_serial.c 524:if (0) ; -- ./arch/powerpc/xmon/ppc-opc.c 938:else if (value == 0) ; -- ./arch/sh/boards/se/7343/io.c 137:if (0) ; -- ./arch/um/kernel/tt/tracer.c 254:if (WIFEXITED(status)) ; -- ./arch/x86_64/ia32/ptrace32.c 363:if (__copy_from_user(&child->thread.i387.fxsave, u, sizeof(*u))) ; -- ./arch/x86_64/kernel/traps.c 801:if (eregs == (struct pt_regs *)eregs->rsp) ; -- ./drivers/atm/iphase.c 159:if (!desc1) ; -- ./drivers/isdn/capi/capiutil.c 456:else if (c <= 0x0f) ; -- ./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)) ; -- ./drivers/media/video/video-buf.c 1141: if (q->bufs[i]) ; -- ./drivers/net/lp486e.c 777:if (lp->scb.command && i596_timeout(dev, "i596_cleanup_cmd", 100)) ; 785:if (lp->scb.command && i596_timeout(dev, "i596_reset", 100)) ; 794:if (lp->scb.command && i596_timeout(dev, "i596_reset(2)", 400)) ; 820:if (lp->scb.command && i596_timeout(dev, "i596_add_cmd", 100)) ; 1146: if (lp->scb.command && i596_timeout(dev, "interrupt", 40)) ; 1192: if (lp->scb.command && i596_timeout(dev, "i596 interrupt", 100)) ; 1217: if (lp->scb.command && i596_timeout(dev, "i596_close", 200)) ; -- ./drivers/net/ni5010.c 273:if (dev->irq == 0xff) ; -- ./drivers/net/ni52.c 648:if (result & TDR_LNK_OK) ; -- ./drivers/net/sun3_82586.c 498:if (result & TDR_LNK_OK) ; -- ./drivers/pci/hotplug/ibmphp_core.c 418:else if (mode == BUS_MODE_PCI) ; 636:else if (mode == BUS_MODE_PCI) ; -- ./drivers/usb/gadget/file_storage.c 2480: if (protocol_is_scsi()) ; -- ./drivers/usb/host/uhci-debug.c 416:if (i <= SKEL_ISO) ; 419:else if (!uhci->fsbr_is_on) ; -- ./drivers/usb/host/uhci-q.c 541:if (qh->skel == SKEL_ISO) ; -- ./drivers/usb/misc/usbtest.c 1401: if (status != 0) ; -- ./drivers/video/intelfb/intelfbdrv.c 337:if (get_opt_bool(this_opt, "accel", &accel)) ; 338:else if (get_opt_int(this_opt, "vram", &vram)) ; 339:else if (get_opt_bool(this_opt, "hwcursor", &hwcursor)) ; 340:else if (get_opt_bool(this_opt, "mtrr", &mtrr)) ; 341:else if (get_opt_bool(this_opt, "fixed", &fixed)) ; -- ./drivers/video/matrox/matroxfb_DAC1064.c 46: if (fvco <= 10) ; -- ./drivers/video/matrox/matroxfb_maven.c 298:if (fvco <= 1) ; 316:if (fvco <= 10) ; -- ./fs/hfs/inode.c 72: if (!node) ; -- ./fs/hfsplus/inode.c 67: if (!node) ; -- ./fs/hostfs/hostfs_user.c 300:if (attrs->ia_valid & HOSTFS_ATTR_CTIME) ; -- ./fs/xfs/xfs_bmap.c 2287: if (nullfb || XFS_FSB_TO_AGNO(mp, ap->rval) == fb_agno) ; -- ./fs/xfs/xfs_dir2.c 281:else if ((rval = xfs_dir2_isblock(tp, dp, &v))) ; -- ./fs/xfs/xfs_iomap.c 248:if (io->io_flags & XFS_IOCORE_RT) ; -- ./include/asm-cris/uaccess.h 255:if (n == 0) ; 303:if (n == 0) ; 351:if (n == 0) ; -- ./mm/swapfile.c 791:if (swcount <= 1) ; -- ./net/core/pktgen.c 2256: if (pkt_dev->min_in6_daddr.s6_addr32[0] == 0 && pkt_dev->min_in6_daddr.s6_addr32[1] == 0 && pkt_dev->min_in6_daddr.s6_addr32[2] == 0 && pkt_dev->min_in6_daddr.s6_addr32[3] == 0) ; -- ./net/irda/af_irda.c 1357: if (ret) ; 1358: else if (sk->sk_shutdown & RCV_SHUTDOWN) ; -- ./net/irda/irnetlink.c 105:if (nla_put_string(msg, IRDA_NL_ATTR_IFNAME, dev->name)) ; -- ./net/netfilter/xt_u32.c 38: if (skb->len < 4 || pos > skb->len - 4) ; -- ./sound/pci/au88x0/au88x0_core.c 2076: if (vortex_adbd
Re: drivers/infiniband/mlx/mad.c misplaced ;
On Wed, Aug 15, 2007 at 05:40:11PM -0700, Joe Perches wrote: > On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote: > > Signed-off-by: Dave Jones <[EMAIL PROTECTED]> > > > > diff --git a/drivers/infiniband/hw/mlx4/mad.c > > b/drivers/infiniband/hw/mlx4/mad.c > > index 3330917..0ed02b7 100644 > > --- a/drivers/infiniband/hw/mlx4/mad.c > > +++ b/drivers/infiniband/hw/mlx4/mad.c > > @@ -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 */ At least this one is not a bug. But I'm going to add a "break" there, so it doesn't look that strange anymore. Thanks! - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
Joe Perches wrote: On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote: Signed-off-by: Dave Jones <[EMAIL PROTECTED]> diff --git a/drivers/infiniband/hw/mlx4/mad.c b/drivers/infiniband/hw/mlx4/mad.c index 3330917..0ed02b7 100644 --- a/drivers/infiniband/hw/mlx4/mad.c +++ b/drivers/infiniband/hw/mlx4/mad.c @@ -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) ; sounds like an excellent candidate check for checkpatch.pl !!! Cheers, Auke - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/infiniband/mlx/mad.c misplaced ;
On Wed, 2007-08-15 at 19:58 -0400, Dave Jones wrote: > Signed-off-by: Dave Jones <[EMAIL PROTECTED]> > > diff --git a/drivers/infiniband/hw/mlx4/mad.c > b/drivers/infiniband/hw/mlx4/mad.c > index 3330917..0ed02b7 100644 > --- a/drivers/infiniband/hw/mlx4/mad.c > +++ b/drivers/infiniband/hw/mlx4/mad.c > @@ -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) ; - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
