[PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Ruan Jinjie]

This patch removes the error checking for debugfs_create_dir in
pcmtest.c. This is because the DebugFS kernel API is developed
in a way that the caller can safely ignore the errors that
occur during the creation of DebugFS nodes. The debugfs APIs have
a IS_ERR() judge in start_creating() which can handle it
gracefully. So these checks are unnecessary.

Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
---
 sound/drivers/pcmtest.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c
index 7f170429eb8f..9360b3bb771e 100644
--- a/sound/drivers/pcmtest.c
+++ b/sound/drivers/pcmtest.c
@@ -669,8 +669,6 @@ static int init_debug_files(int buf_count)
 	char len_file_name[32];
 
 	driver_debug_dir = debugfs_create_dir("pcmtest", NULL);
-	if (IS_ERR(driver_debug_dir))
-		return PTR_ERR(driver_debug_dir);
 	debugfs_create_u8("pc_test", 0444, driver_debug_dir, &playback_capture_test);
 	debugfs_create_u8("ioctl_test", 0444, driver_debug_dir, &ioctl_reset_test);
 
-- 
2.34.1

Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Takashi Iwai]

On Thu, 17 Aug 2023 08:39:22 +0200,
Ruan Jinjie wrote:
> 
> This patch removes the error checking for debugfs_create_dir in
> pcmtest.c. This is because the DebugFS kernel API is developed
> in a way that the caller can safely ignore the errors that
> occur during the creation of DebugFS nodes. The debugfs APIs have
> a IS_ERR() judge in start_creating() which can handle it
> gracefully. So these checks are unnecessary.
> 
> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>

I believe debugfs is mandatory in this case (it's a test module that
manipulates / gets notification via debugfs), hence it makes sense to
check the error.

Maybe we should add a dependency on CONFIG_DEBUG_FS in Kconfig?


thanks,

Takashi

> ---
>  sound/drivers/pcmtest.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c
> index 7f170429eb8f..9360b3bb771e 100644
> --- a/sound/drivers/pcmtest.c
> +++ b/sound/drivers/pcmtest.c
> @@ -669,8 +669,6 @@ static int init_debug_files(int buf_count)
>  	char len_file_name[32];
>  
>  	driver_debug_dir = debugfs_create_dir("pcmtest", NULL);
> -	if (IS_ERR(driver_debug_dir))
> -		return PTR_ERR(driver_debug_dir);
>  	debugfs_create_u8("pc_test", 0444, driver_debug_dir, &playback_capture_test);
>  	debugfs_create_u8("ioctl_test", 0444, driver_debug_dir, &ioctl_reset_test);
>  
> -- 
> 2.34.1
> 

Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Ruan Jinjie]

On 2023/8/17 14:47, Takashi Iwai wrote:
> On Thu, 17 Aug 2023 08:39:22 +0200,
> Ruan Jinjie wrote:
>>
>> This patch removes the error checking for debugfs_create_dir in
>> pcmtest.c. This is because the DebugFS kernel API is developed
>> in a way that the caller can safely ignore the errors that
>> occur during the creation of DebugFS nodes. The debugfs APIs have
>> a IS_ERR() judge in start_creating() which can handle it
>> gracefully. So these checks are unnecessary.
>>
>> Signed-off-by: Ruan Jinjie <ruanjinjie@huawei.com>
> 
> I believe debugfs is mandatory in this case (it's a test module that
> manipulates / gets notification via debugfs), hence it makes sense to
> check the error.

So the error check is necessary!

> 
> Maybe we should add a dependency on CONFIG_DEBUG_FS in Kconfig?

I think it's ok!

> 
> 
> thanks,
> 
> Takashi
> 
>> ---
>>  sound/drivers/pcmtest.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/sound/drivers/pcmtest.c b/sound/drivers/pcmtest.c
>> index 7f170429eb8f..9360b3bb771e 100644
>> --- a/sound/drivers/pcmtest.c
>> +++ b/sound/drivers/pcmtest.c
>> @@ -669,8 +669,6 @@ static int init_debug_files(int buf_count)
>>  	char len_file_name[32];
>>  
>>  	driver_debug_dir = debugfs_create_dir("pcmtest", NULL);
>> -	if (IS_ERR(driver_debug_dir))
>> -		return PTR_ERR(driver_debug_dir);
>>  	debugfs_create_u8("pc_test", 0444, driver_debug_dir, &playback_capture_test);
>>  	debugfs_create_u8("ioctl_test", 0444, driver_debug_dir, &ioctl_reset_test);
>>  
>> -- 
>> 2.34.1
>>

Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Ivan Orlov]

On 17.08.2023 11:03, Ruan Jinjie wrote:
> So the error check is necessary!

Yeah, it is. As Takashi already mentioned, debugfs in this case is the 
way how the driver communicates with userspace (forwards flags, receives 
filling patterns, etc.), so it is vital part of the driver.

> 
>>
>> Maybe we should add a dependency on CONFIG_DEBUG_FS in Kconfig?
> 
> I think it's ok!

It sounds like a great idea. Ruan, would you like to do this? If not, I 
will take it on.

--
Kind regards,
Ivan Orlov

Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Ruan Jinjie]

On 2023/8/17 16:30, Ivan Orlov wrote:
> On 17.08.2023 11:03, Ruan Jinjie wrote:
>> So the error check is necessary!
> 
> Yeah, it is. As Takashi already mentioned, debugfs in this case is the
> way how the driver communicates with userspace (forwards flags, receives
> filling patterns, etc.), so it is vital part of the driver.
> 
>>
>>>
>>> Maybe we should add a dependency on CONFIG_DEBUG_FS in Kconfig?
>>
>> I think it's ok!
> 
> It sounds like a great idea. Ruan, would you like to do this? If not, I
> will take it on.

Yes, I'd like to. I'll send it sooner. Thank you very much!

> 
> -- 
> Kind regards,
> Ivan Orlov
> 

Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir

[Ivan Orlov]

On 17.08.2023 12:33, Ruan Jinjie wrote:

> Yes, I'd like to. I'll send it sooner. Thank you very much!

Alright, thank you for doing this :)