Message ID | alpine.DEB.2.10.1512231352410.4367@hadrien |
---|---|
State | Not Applicable |
Delegated to: | Jeff Kirsher |
Headers | show |
Actully, there is a more serious problem here, that there are a lot of potential dereferences of invalid pointers, via calls like: dev_err(&adapter->pdev->dev, "map to unbound device!\n") julia On Wed, 23 Dec 2015, Julia Lawall wrote: > Kernel code typically uses == NULL. > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > CC: Gangfeng Huang <gangfeng.huang@ni.com> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > > --- > > Alternatively, consider using !adapter. > > igb_cdev.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > --- a/drivers/net/ethernet/intel/igb/igb_cdev.c > +++ b/drivers/net/ethernet/intel/igb/igb_cdev.c > @@ -92,7 +92,7 @@ static int igb_bind(struct file *file, v > > adapter = (struct igb_adapter *)file->private_data; > > - if (NULL == adapter) > + if (adapter == NULL) > return -ENOENT; > > mmap_size = pci_resource_len(adapter->pdev, 0); > @@ -119,7 +119,7 @@ static long igb_mapring(struct file *fil > return -EINVAL; > > adapter = file->private_data; > - if (NULL == adapter) { > + if (adapter == NULL) { > dev_err(&adapter->pdev->dev, "map to unbound device!\n"); > return -ENOENT; > } > @@ -182,7 +182,7 @@ static long igb_mapbuf(struct file *file > return -EINVAL; > > adapter = file->private_data; > - if (NULL == adapter) { > + if (adapter == NULL) { > dev_err(&adapter->pdev->dev, "map to unbound device!\n"); > return -ENOENT; > } > @@ -246,7 +246,7 @@ static long igb_unmapring(struct file *f > return -EINVAL; > > adapter = file->private_data; > - if (NULL == adapter) { > + if (adapter == NULL) { > dev_err(&adapter->pdev->dev, "map to unbound device!\n"); > return -ENOENT; > } > @@ -310,7 +310,7 @@ static long igb_unmapbuf(struct file *fi > return -EFAULT; > > adapter = file->private_data; > - if (NULL == adapter) { > + if (adapter == NULL) { > dev_err(&adapter->pdev->dev, "map to unbound device!\n"); > return -ENOENT; > } > @@ -398,7 +398,7 @@ static int igb_close_file(struct inode * > { > struct igb_adapter *adapter = file->private_data; > > - if (NULL == adapter) > + if (adapter == NULL) > return 0; > > mutex_lock(&adapter->cdev_mutex); > @@ -434,7 +434,7 @@ static int igb_mmap(struct file *file, s > dma_addr_t pgoff = vma->vm_pgoff; > dma_addr_t physaddr; > > - if (NULL == adapter) > + if (adapter == NULL) > return -ENODEV; > > if (pgoff == 0) >
> From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on behalf of Julia Lawall [julia.lawall@lip6.fr] > Sent: Wednesday, December 23, 2015 4:58 AM > To: Gangfeng Huang > Cc: intel-wired-lan@lists.osuosl.org; kbuild-all@01.org > Subject: Re: [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings > > Actully, there is a more serious problem here, that there are a lot of > potential dereferences of invalid pointers, via calls like: > > dev_err(&adapter->pdev->dev, "map to unbound device!\n") > julia I don't see that this patch changes that, it still seems proper to apply this clean up. > On Wed, 23 Dec 2015, Julia Lawall wrote: > > > Kernel code typically uses == NULL. > > > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > > > CC: Gangfeng Huang <gangfeng.huang@ni.com> > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > > > > --- > > > > Alternatively, consider using !adapter. > > > > igb_cdev.c | 14 +++++++------- > > 1 file changed, 7 insertions(+), 7 deletions(-) Tested-by: Aaron Brown <aaron.f.brown@intel.com>
On Thu, 7 Jan 2016, Brown, Aaron F wrote: > > From: Intel-wired-lan [intel-wired-lan-bounces@lists.osuosl.org] on behalf of Julia Lawall [julia.lawall@lip6.fr] > > Sent: Wednesday, December 23, 2015 4:58 AM > > To: Gangfeng Huang > > Cc: intel-wired-lan@lists.osuosl.org; kbuild-all@01.org > > Subject: Re: [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings > > > > Actully, there is a more serious problem here, that there are a lot of > > potential dereferences of invalid pointers, via calls like: > > > > dev_err(&adapter->pdev->dev, "map to unbound device!\n") > > > julia > > I don't see that this patch changes that, it still seems proper to apply this clean up. No, the patch just does what the rule says, which is move the NULL to the other side. julia > > > On Wed, 23 Dec 2015, Julia Lawall wrote: > > > > > Kernel code typically uses == NULL. > > > > > > Generated by: scripts/coccinelle/misc/compare_const_fl.cocci > > > > > > CC: Gangfeng Huang <gangfeng.huang@ni.com> > > > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com> > > > Signed-off-by: Julia Lawall <julia.lawall@lip6.fr> > > > > > > --- > > > > > > Alternatively, consider using !adapter. > > > > > > igb_cdev.c | 14 +++++++------- > > > 1 file changed, 7 insertions(+), 7 deletions(-) > > Tested-by: Aaron Brown <aaron.f.brown@intel.com>
--- a/drivers/net/ethernet/intel/igb/igb_cdev.c +++ b/drivers/net/ethernet/intel/igb/igb_cdev.c @@ -92,7 +92,7 @@ static int igb_bind(struct file *file, v adapter = (struct igb_adapter *)file->private_data; - if (NULL == adapter) + if (adapter == NULL) return -ENOENT; mmap_size = pci_resource_len(adapter->pdev, 0); @@ -119,7 +119,7 @@ static long igb_mapring(struct file *fil return -EINVAL; adapter = file->private_data; - if (NULL == adapter) { + if (adapter == NULL) { dev_err(&adapter->pdev->dev, "map to unbound device!\n"); return -ENOENT; } @@ -182,7 +182,7 @@ static long igb_mapbuf(struct file *file return -EINVAL; adapter = file->private_data; - if (NULL == adapter) { + if (adapter == NULL) { dev_err(&adapter->pdev->dev, "map to unbound device!\n"); return -ENOENT; } @@ -246,7 +246,7 @@ static long igb_unmapring(struct file *f return -EINVAL; adapter = file->private_data; - if (NULL == adapter) { + if (adapter == NULL) { dev_err(&adapter->pdev->dev, "map to unbound device!\n"); return -ENOENT; } @@ -310,7 +310,7 @@ static long igb_unmapbuf(struct file *fi return -EFAULT; adapter = file->private_data; - if (NULL == adapter) { + if (adapter == NULL) { dev_err(&adapter->pdev->dev, "map to unbound device!\n"); return -ENOENT; } @@ -398,7 +398,7 @@ static int igb_close_file(struct inode * { struct igb_adapter *adapter = file->private_data; - if (NULL == adapter) + if (adapter == NULL) return 0; mutex_lock(&adapter->cdev_mutex); @@ -434,7 +434,7 @@ static int igb_mmap(struct file *file, s dma_addr_t pgoff = vma->vm_pgoff; dma_addr_t physaddr; - if (NULL == adapter) + if (adapter == NULL) return -ENODEV; if (pgoff == 0)