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