| Message ID | 20230817063922.282746-1-ruanjinjie@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] ALSA: pcmtest: Drop unnecessary error check for debugfs_create_dir | expand |
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 >>
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 >
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 :)
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);
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(-)