Re: [PATCH] iio: core: print error message on debugfs register failure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 11, 2019 at 05:16:36PM +0200, Alexandru Ardelean wrote:
> From: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> 
> If there's a failure when registering a debugfs entry for a device, don't
> silently ignore the failure. Instead, print an error message and an error
> code signaling the failure.

No, never do that.

> Signed-off-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
> ---
>  drivers/iio/industrialio-core.c | 34 +++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index dab67cb69fe6..662dabf8b08c 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -364,23 +364,45 @@ static const struct file_operations iio_debugfs_reg_fops = {
>  static void iio_device_unregister_debugfs(struct iio_dev *indio_dev)
>  {
>  	debugfs_remove_recursive(indio_dev->debugfs_dentry);
> +	indio_dev->debugfs_dentry = NULL;
>  }
>  
>  static void iio_device_register_debugfs(struct iio_dev *indio_dev)
>  {
> +	struct dentry *d;
> +	int ret;
> +
>  	if (indio_dev->info->debugfs_reg_access == NULL)
>  		return;
>  
>  	if (!iio_debugfs_dentry)
>  		return;
>  
> -	indio_dev->debugfs_dentry =
> -		debugfs_create_dir(dev_name(&indio_dev->dev),
> -				   iio_debugfs_dentry);
> +	d = debugfs_create_dir(dev_name(&indio_dev->dev), iio_debugfs_dentry);
> +	if (IS_ERR_OR_NULL(d))
> +		goto error;

No, don't do that, I spent a lot of time removing all of these pointless
checks.

No kernel code shoudl ever care if debugfs is workign or not, just make
the call and move on.   You can always pass the result of a debugfs call
into another one with no problems.

> +
> +	indio_dev->debugfs_dentry = d;
> +
> +	d = debugfs_create_file("direct_reg_access", 0644,
> +				indio_dev->debugfs_dentry, indio_dev,
> +				&iio_debugfs_reg_fops);
> +
> +	if (IS_ERR_OR_NULL(d))
> +		goto error;

This check isn't even correct :)

So this isn't going to work no matter what, sorry.

just don't do this.

The code is just fine as-is.

thanks,

greg k-h



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux