Re: [PATCH] cxl: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Jun 12, 2019 at 11:51:21AM +0200, Arnd Bergmann wrote:
> On Tue, Jun 11, 2019 at 8:13 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> 
> > @@ -64,8 +64,6 @@ int cxl_debugfs_adapter_add(struct cxl *adapter)
> >
> >         snprintf(buf, 32, "card%i", adapter->adapter_num);
> >         dir = debugfs_create_dir(buf, cxl_debugfs);
> > -       if (IS_ERR(dir))
> > -               return PTR_ERR(dir);
> >         adapter->debugfs = dir;
> >
> 
> Should the check for 'cxl_debugfs' get removed here as well?

Maybe, I could not determine the logic if those functions could be
called before cxl_debugfs was ever set.

And debugfs_create_dir() will not return a NULL value if an error
happens, so no need to worry about files being created in the wrong
place.

> If that is null, we might put the subdir in the wrong place in the
> tree, but that would otherwise be harmless as well, and the
> same thing happens if 'dir' is NULL above and we add the
> files in the debugfs root later (losing the ability to clean up
> afterwards).
> 
> int cxl_debugfs_adapter_add(struct cxl *adapter)
> {
>         struct dentry *dir;
>         char buf[32];
> 
>         if (!cxl_debugfs)
>                 return -ENODEV;
> 
> It's still a bit odd to return an error, since the caller then just
> ignores the return code anway:

Then let's just return nothing.

>         /* Don't care if this one fails: */
>         cxl_debugfs_adapter_add(adapter);
> 
> It would seem best to change the return type to 'void' here for
> consistency.

I agree, let me go do that.

thanks,

greg k-h