* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
@ 2015-12-23 12:53 Julia Lawall
2015-12-23 12:58 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2015-12-23 12:53 UTC (permalink / raw)
To: intel-wired-lan
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
2015-12-23 12:53 [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings Julia Lawall
@ 2015-12-23 12:58 ` Julia Lawall
2016-01-07 3:12 ` Brown, Aaron F
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2015-12-23 12:58 UTC (permalink / raw)
To: intel-wired-lan
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)
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
2015-12-23 12:58 ` Julia Lawall
@ 2016-01-07 3:12 ` Brown, Aaron F
2016-01-07 6:22 ` Julia Lawall
0 siblings, 1 reply; 6+ messages in thread
From: Brown, Aaron F @ 2016-01-07 3:12 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Julia Lawall [julia.lawall at lip6.fr]
> Sent: Wednesday, December 23, 2015 4:58 AM
> To: Gangfeng Huang
> Cc: intel-wired-lan at lists.osuosl.org; kbuild-all at 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
2016-01-07 3:12 ` Brown, Aaron F
@ 2016-01-07 6:22 ` Julia Lawall
0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2016-01-07 6:22 UTC (permalink / raw)
To: intel-wired-lan
On Thu, 7 Jan 2016, Brown, Aaron F wrote:
> > From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Julia Lawall [julia.lawall at lip6.fr]
> > Sent: Wednesday, December 23, 2015 4:58 AM
> > To: Gangfeng Huang
> > Cc: intel-wired-lan at lists.osuosl.org; kbuild-all at 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
2015-12-14 15:00 Julia Lawall
@ 2015-12-18 2:58 ` Brown, Aaron F
0 siblings, 0 replies; 6+ messages in thread
From: Brown, Aaron F @ 2015-12-18 2:58 UTC (permalink / raw)
To: intel-wired-lan
> From: Intel-wired-lan [intel-wired-lan-bounces at lists.osuosl.org] on behalf of Julia Lawall [julia.lawall at lip6.fr]
> Sent: Monday, December 14, 2015 7:00 AM
> To: Gangfeng Huang
> Cc: intel-wired-lan at lists.osuosl.org; kbuild-all at 01.org
> Subject: [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
>
> In both of these cases, it is more common to have the constant on the
> right, in kernel code.
>
> 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>
>
> ---
>
> igb_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
And based on another patch I was looking at checkpatch agrees :)
It also suggests as a check that this could be written !adapter rather than NULL == adapter
------------------------------------------------------------------------------------------------
u1458:[1]/usr/src/kernels/next-queue> git format-patch 1b199aa -1 --stdout|./scripts/checkpatch.pl -
CHECK: Comparison to NULL could be written "!adapter"
#27: FILE: drivers/net/ethernet/intel/igb/igb_main.c:8276:
+ if (adapter == NULL) {
total: 0 errors, 0 warnings, 1 checks, 16 lines checked
Your patch has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
u1458:[1]/usr/src/kernels/next-queue>
------------------------------------------------------------------------------------------------
But I don't have particularly strong feelings on that either way, so...
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings
@ 2015-12-14 15:00 Julia Lawall
2015-12-18 2:58 ` Brown, Aaron F
0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2015-12-14 15:00 UTC (permalink / raw)
To: intel-wired-lan
In both of these cases, it is more common to have the constant on the
right, in kernel code.
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>
---
igb_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -8209,7 +8209,7 @@ static int igb_change_mode(struct igb_ad
int err = 0;
int current_mode;
- if (NULL == adapter) {
+ if (adapter == NULL) {
dev_err(&adapter->pdev->dev, "map to unbound device!\n");
return -ENOENT;
}
@@ -8272,7 +8272,7 @@ static ssize_t igb_set_qav_mode(struct d
if (!capable(CAP_NET_ADMIN))
return -EPERM;
- if (0 > kstrtoint(buf, 0, &request_mode))
+ if (kstrtoint(buf, 0, &request_mode) < 0)
return -EINVAL;
if (request_mode != 0 && request_mode != 1)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-07 6:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23 12:53 [Intel-wired-lan] [PATCH] igb: fix compare_const_fl.cocci warnings Julia Lawall
2015-12-23 12:58 ` Julia Lawall
2016-01-07 3:12 ` Brown, Aaron F
2016-01-07 6:22 ` Julia Lawall
-- strict thread matches above, loose matches on Subject: below --
2015-12-14 15:00 Julia Lawall
2015-12-18 2:58 ` Brown, Aaron F
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.