[PATCH] s390/pci: Remove unnecessary if condition

[PATCH] s390/pci: Remove unnecessary if condition

[Amitoj Kaur Chawla]

Remove unnecessary error handling because the only failure value that
can be returned is NULL and so the test can never be true.

The Coccinelle semantic patch used to make this change is as follows:
@@
expression e;
@@

  e = debugfs_create_file(...);
- if(IS_ERR(e)) { e = NULL; }

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
---
 arch/s390/pci/pci_debug.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/s390/pci/pci_debug.c b/arch/s390/pci/pci_debug.c
index 4129b0a..8a25b04 100644
--- a/arch/s390/pci/pci_debug.c
+++ b/arch/s390/pci/pci_debug.c
@@ -139,8 +139,6 @@ void zpci_debug_init_device(struct zpci_dev *zdev)
 				S_IFREG | S_IRUGO | S_IWUSR,
 				zdev->debugfs_dev, zdev,
 				&debugfs_pci_perf_fops);
-	if (IS_ERR(zdev->debugfs_perf))
-		zdev->debugfs_perf = NULL;
 }
 
 void zpci_debug_exit_device(struct zpci_dev *zdev)
-- 
1.9.1

Re: [PATCH] s390/pci: Remove unnecessary if condition

[Sebastian Ott]

On Fri, 29 Jul 2016, Amitoj Kaur Chawla wrote:
> Remove unnecessary error handling because the only failure value that
> can be returned is NULL and so the test can never be true.
> 
> The Coccinelle semantic patch used to make this change is as follows:
> @@
> expression e;
> @@
> 
>   e = debugfs_create_file(...);
> - if(IS_ERR(e)) { e = NULL; }

Nope. For !CONFIG_DEBUG_FS debugfs_create_file returns an ERR_PTR.

Regards,
Sebastian

Re: [PATCH] s390/pci: Remove unnecessary if condition

[Julia Lawall]

On Fri, 29 Jul 2016, Sebastian Ott wrote:

> On Fri, 29 Jul 2016, Amitoj Kaur Chawla wrote:
> > Remove unnecessary error handling because the only failure value that
> > can be returned is NULL and so the test can never be true.
> >
> > The Coccinelle semantic patch used to make this change is as follows:
> > @@
> > expression e;
> > @@
> >
> >   e = debugfs_create_file(...);
> > - if(IS_ERR(e)) { e = NULL; }
>
> Nope. For !CONFIG_DEBUG_FS debugfs_create_file returns an ERR_PTR.

Clicking around in lxr doesn't show that, but perhaps an alternative
definition is overlooked?

julia

Re: [PATCH] s390/pci: Remove unnecessary if condition

[Sebastian Ott]

On Fri, 29 Jul 2016, Julia Lawall wrote:
> On Fri, 29 Jul 2016, Sebastian Ott wrote:
> > On Fri, 29 Jul 2016, Amitoj Kaur Chawla wrote:
> > > Remove unnecessary error handling because the only failure value that
> > > can be returned is NULL and so the test can never be true.
> > >
> > > The Coccinelle semantic patch used to make this change is as follows:
> > > @@
> > > expression e;
> > > @@
> > >
> > >   e = debugfs_create_file(...);
> > > - if(IS_ERR(e)) { e = NULL; }
> >
> > Nope. For !CONFIG_DEBUG_FS debugfs_create_file returns an ERR_PTR.
> 
> Clicking around in lxr doesn't show that, but perhaps an alternative
> definition is overlooked?

It looks that way.


[sebott@schleppi linux]$ git grep -W "debugfs_create_file(" include/linux/debugfs.h
include/linux/debugfs.h:struct dentry *debugfs_create_file(const char *name, umode_t mode,
include/linux/debugfs.h-                                   struct dentry *parent, void *data,
include/linux/debugfs.h-                                   const struct file_operations *fops);
--
include/linux/debugfs.h:static inline struct dentry *debugfs_create_file(const char *name, umode_t mode,
include/linux/debugfs.h-                                        struct dentry *parent, void *data,
include/linux/debugfs.h-                                        const struct file_operations *fops)
include/linux/debugfs.h-{
include/linux/debugfs.h-        return ERR_PTR(-ENODEV);
include/linux/debugfs.h-}
include/linux/debugfs.h-

Regards,
Sebastian

Re: [PATCH] s390/pci: Remove unnecessary if condition

[Julia Lawall]

On Fri, 29 Jul 2016, Sebastian Ott wrote:

> On Fri, 29 Jul 2016, Julia Lawall wrote:
> > On Fri, 29 Jul 2016, Sebastian Ott wrote:
> > > On Fri, 29 Jul 2016, Amitoj Kaur Chawla wrote:
> > > > Remove unnecessary error handling because the only failure value that
> > > > can be returned is NULL and so the test can never be true.
> > > >
> > > > The Coccinelle semantic patch used to make this change is as follows:
> > > > @@
> > > > expression e;
> > > > @@
> > > >
> > > >   e = debugfs_create_file(...);
> > > > - if(IS_ERR(e)) { e = NULL; }
> > >
> > > Nope. For !CONFIG_DEBUG_FS debugfs_create_file returns an ERR_PTR.
> >
> > Clicking around in lxr doesn't show that, but perhaps an alternative
> > definition is overlooked?
>
> It looks that way.
>
>
> [sebott@schleppi linux]$ git grep -W "debugfs_create_file(" include/linux/debugfs.h
> include/linux/debugfs.h:struct dentry *debugfs_create_file(const char *name, umode_t mode,
> include/linux/debugfs.h-                                   struct dentry *parent, void *data,
> include/linux/debugfs.h-                                   const struct file_operations *fops);
> --
> include/linux/debugfs.h:static inline struct dentry *debugfs_create_file(const char *name, umode_t mode,
> include/linux/debugfs.h-                                        struct dentry *parent, void *data,
> include/linux/debugfs.h-                                        const struct file_operations *fops)
> include/linux/debugfs.h-{
> include/linux/debugfs.h-        return ERR_PTR(-ENODEV);
> include/linux/debugfs.h-}
> include/linux/debugfs.h-

Thanks for the pointer.  I guess that if the result of debugfs_create_file
is actully dereferenced, then both test should be present, eg:

drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c

       ent = debugfs_create_file(name,
                                 S_IFREG | S_IRUGO, root,
                                 ring, &amdgpu_debugfs_ring_fops);
       if (IS_ERR(ent))
		return PTR_ERR(ent);

       i_size_write(ent->d_inode, ring->ring_size + 12);

This problem seems to only occur in this directory.

thanks,
julia