Re: [PATCH -next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir
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);-- 2.34.1
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);-- 2.34.1
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
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
participants (2)
-
Ivan Orlov -
Ruan Jinjie