Re: [PATCH 5/5] sound: soc: core: no need to check return value of debugfs_create functions

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1410 bytes --]

On Fri, Jun 14, 2019 at 11:47:56AM +0200, Greg Kroah-Hartman wrote:

> Note, the soc-pcm "state" file has now moved to a subdirectory, as it is
> only a good idea to save the dentries for debugfs directories, not
> individual files, as the individual file debugfs functions are changing
> to not return a dentry.

It'd be better to split this out into a separate change for ease of
review.

> -	d = debugfs_create_file(w->name, 0444,
> -				dapm->debugfs_dapm, w,
> -				&dapm_widget_power_fops);
> -	if (!d)
> -		dev_warn(w->dapm->dev,
> -			"ASoC: Failed to create %s debugfs file\n",
> -			w->name);
> +	debugfs_create_file(w->name, 0444, dapm->debugfs_dapm, w,
> +			    &dapm_widget_power_fops);

The majority of this is removing error prints rather than code that
actively does something different.  If this was like kmalloc() where the
API is itself reported errors then this wouldn't be an issue but unless
I'm missing something debugfs fails silently so this means that if
something goes wrong it's going to be harder for the user to figure out
where the debugfs files they wanted to check went to.  I'm guessing you
don't want to add error prints in debugfs itself so I'd rather they
stayed here.

Yes, the error check is looking for NULL not an error pointer - it was
correct when written but I see that the debugfs API changed earlier this
year to return error pointers so we ought to fix that up.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]