diff mbox

igb: fix compare_const_fl.cocci warnings

Message ID alpine.DEB.2.10.1512231352410.4367@hadrien
State Not Applicable
Delegated to: Jeff Kirsher
Headers show

Commit Message

Julia Lawall Dec. 23, 2015, 12:53 p.m. UTC
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(-)

Comments

Julia Lawall Dec. 23, 2015, 12:58 p.m. UTC | #1
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)
>
Brown, Aaron F Jan. 7, 2016, 3:12 a.m. UTC | #2
> 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>
Julia Lawall Jan. 7, 2016, 6:22 a.m. UTC | #3
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>
diff mbox

Patch

--- 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)