Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1539909

Re: [patch] nvme-fabrics: correct some printk information

Show all headers | View raw

On Sat, 2016-12-10 at 21:06 +0100, Julia Lawall wrote:
> 
> On Sat, 10 Dec 2016, Dan Carpenter wrote:
> 
> > On Sat, Dec 10, 2016 at 03:27:50AM -0800, Joe Perches wrote:
> > > On Sat, 2016-12-10 at 12:06 +0300, Dan Carpenter wrote:
> > > > We really don't care where "ctrl" is on the stack since we're just
> > > > returning soon what we want is the actual ctrl pointer itself.
> > > > 
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > 
> > > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> > > 
> > > []
> > > > @@ -2402,7 +2402,7 @@ enum blk_eh_timer_return
> > > > 
> > > >  	dev_info(ctrl->ctrl.device,
> > > >  		"NVME-FC{%d}: new ctrl: NQN \"%s\" (%p)\n",
> > > > -		ctrl->cnum, ctrl->ctrl.opts->subsysnqn, &ctrl);
> > > > +		ctrl->cnum, ctrl->ctrl.opts->subsysnqn, ctrl);
> > > 
> > > Found by script or inspection?
> > > 
> > > If by script, it seems unlikely there's only 1 instance
> > > where an address of an automatic pointer type is used
> > > incorrectly.
> > 
> > Script.  But it's using a pretty specific heuristic where we kmalloc a
> > pointer and then pass the address.  It prints few warnings.  Probably
> > 40% false positives, but the remaining examples of course are 100% false
> > positives.
> 
> I tried anything that looks like a print, ie has a format string argument,
> and was taking the address of a local variable as another argument.  But
> there are lots of weird format designators in the kernel that Coccinelle
> doesn't know about for which passing the address of a local variable is
> reasonable.  So for the moment, there are, as far as I can see, just a lot
> of false positives.  I did add improving the support for format strings to
> my TODO list.

I think there's probably a class of defects that could
be found something like this in coccinelle:

@@
type T;
T *t;
@@

* \(netdev_emerg\|netdev_crit\|netdev_alert\|netdev_err\|netdev_notice\|netdev_warn\|netdev_warn\|netdev_info\|netdev_dbg\|dev_emerg\|dev_crit\|dev_alert\|dev_err\|dev_notice\|dev_warn\|dev_warn\|dev_info\|dev_dbg\|pr_emerg\|pr_crit\|pr_alert\|pr_err\|pr_notice\|pr_warn\|pr_warning\|pr_warn\|pr_info\|pr_debug\|printk\|vsprintf\|vscnprintf\|vsprintf\)(..., &t, ...);

This finds a few like:

diff -u -p drivers//dma/pxa_dma.c /tmp/nothing//dma/pxa_dma.c
--- drivers//dma/pxa_dma.c
+++ /tmp/nothing//dma/pxa_dma.c
@@ -640,9 +640,6 @@ static unsigned int clear_chan_irq(struc
 	dcsr = phy_readl_relaxed(phy, DCSR);
 	phy_writel(phy, dcsr, DCSR);
 	if ((dcsr & PXA_DCSR_BUSERR) && (phy->vchan))
-		dev_warn(&phy->vchan->vc.chan.dev->device,
-			 "%s(chan=%p): PXA_DCSR_BUSERR\n",
-			 __func__, &phy->vchan);
 
 	return dcsr & ~PXA_DCSR_RUN;
 }

btw:  It'd be nice if coccinelle could use multiple nested "\("  

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-10 10:20 +0100
  Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-10 12:30 +0100
    Re: [patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-10 20:00 +0100
      Re: [patch] nvme-fabrics: correct some printk information Julia Lawall <julia.lawall@lip6.fr> - 2016-12-10 21:10 +0100
        Re: [patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-10 21:30 +0100
        Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-10 22:00 +0100
          Re: [patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-10 22:10 +0100
            Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-10 23:30 +0100
              Re: [patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-12 10:40 +0100
                Re: [patch] nvme-fabrics: correct some printk information Julia Lawall <julia.lawall@lip6.fr> - 2016-12-12 16:50 +0100
                Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-12 17:00 +0100
          Re: [patch] nvme-fabrics: correct some printk information Julia Lawall <julia.lawall@lip6.fr> - 2016-12-10 23:10 +0100
            Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-10 23:30 +0100
        Re: [patch] nvme-fabrics: correct some printk information Joe Perches <joe@perches.com> - 2016-12-11 01:40 +0100
  Re: [patch] nvme-fabrics: correct some printk information James Smart <james.smart@broadcom.com> - 2016-12-20 01:50 +0100
    Re: [patch] nvme-fabrics: correct some printk information Dan Carpenter <dan.carpenter@oracle.com> - 2016-12-20 10:20 +0100

csiph-web