* [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions
@ 2019-06-12 15:35 Greg Kroah-Hartman
2019-06-13 14:33 ` Jason Baron
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 15:35 UTC (permalink / raw)
To: Jason Baron; +Cc: linux-kernel
When calling debugfs functions, there is no need to ever check the
return value. The function can work or not, but the code logic should
never do something different based on this.
Cc: Jason Baron <jbaron@akamai.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
lib/dynamic_debug.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 8a16c2d498e9..c60409138e13 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -993,20 +993,14 @@ static __initdata int ddebug_init_success;
static int __init dynamic_debug_init_debugfs(void)
{
- struct dentry *dir, *file;
+ struct dentry *dir;
if (!ddebug_init_success)
return -ENODEV;
dir = debugfs_create_dir("dynamic_debug", NULL);
- if (!dir)
- return -ENOMEM;
- file = debugfs_create_file("control", 0644, dir, NULL,
- &ddebug_proc_fops);
- if (!file) {
- debugfs_remove(dir);
- return -ENOMEM;
- }
+ debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
+
return 0;
}
--
2.22.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions
2019-06-12 15:35 [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-06-13 14:33 ` Jason Baron
2019-06-13 15:59 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Jason Baron @ 2019-06-13 14:33 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel
On 6/12/19 11:35 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> lib/dynamic_debug.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 8a16c2d498e9..c60409138e13 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -993,20 +993,14 @@ static __initdata int ddebug_init_success;
>
> static int __init dynamic_debug_init_debugfs(void)
> {
> - struct dentry *dir, *file;
> + struct dentry *dir;
>
> if (!ddebug_init_success)
> return -ENODEV;
>
> dir = debugfs_create_dir("dynamic_debug", NULL);
> - if (!dir)
> - return -ENOMEM;
> - file = debugfs_create_file("control", 0644, dir, NULL,
> - &ddebug_proc_fops);
> - if (!file) {
> - debugfs_remove(dir);
> - return -ENOMEM;
> - }
> + debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
> +
> return 0;
> }
>
>
Looks like debugfs_create_dir() can return NULL, and in that case if its
passed to debugfs_create_file() then the 'control' file ends up in the
root of debugfs? I think its better to just not create the file then
have it in the wrong place so maybe the file creation should be guarded
by if(IS_ERR_OR_NULL(dir)).
Thanks,
-Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions
2019-06-13 14:33 ` Jason Baron
@ 2019-06-13 15:59 ` Greg Kroah-Hartman
2019-06-13 16:09 ` Jason Baron
0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-13 15:59 UTC (permalink / raw)
To: Jason Baron; +Cc: linux-kernel
On Thu, Jun 13, 2019 at 10:33:23AM -0400, Jason Baron wrote:
> On 6/12/19 11:35 AM, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Cc: Jason Baron <jbaron@akamai.com>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > lib/dynamic_debug.c | 12 +++---------
> > 1 file changed, 3 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index 8a16c2d498e9..c60409138e13 100644
> > --- a/lib/dynamic_debug.c
> > +++ b/lib/dynamic_debug.c
> > @@ -993,20 +993,14 @@ static __initdata int ddebug_init_success;
> >
> > static int __init dynamic_debug_init_debugfs(void)
> > {
> > - struct dentry *dir, *file;
> > + struct dentry *dir;
> >
> > if (!ddebug_init_success)
> > return -ENODEV;
> >
> > dir = debugfs_create_dir("dynamic_debug", NULL);
> > - if (!dir)
> > - return -ENOMEM;
> > - file = debugfs_create_file("control", 0644, dir, NULL,
> > - &ddebug_proc_fops);
> > - if (!file) {
> > - debugfs_remove(dir);
> > - return -ENOMEM;
> > - }
> > + debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
> > +
> > return 0;
> > }
> >
> >
>
> Looks like debugfs_create_dir() can return NULL,
No it can not.
> and in that case if its passed to debugfs_create_file() then the
> 'control' file ends up in the root of debugfs?
If it could, yes, that is what would happen.
> I think its better to just not create the file then have it in the
> wrong place so maybe the file creation should be guarded by
> if(IS_ERR_OR_NULL(dir)).
As debugfs_create_dir() can not return NULL, you don't have to worry
about this :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions
2019-06-13 15:59 ` Greg Kroah-Hartman
@ 2019-06-13 16:09 ` Jason Baron
2019-06-13 17:41 ` Greg Kroah-Hartman
0 siblings, 1 reply; 5+ messages in thread
From: Jason Baron @ 2019-06-13 16:09 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel
On 6/13/19 11:59 AM, Greg Kroah-Hartman wrote:
> On Thu, Jun 13, 2019 at 10:33:23AM -0400, Jason Baron wrote:
>> On 6/12/19 11:35 AM, Greg Kroah-Hartman wrote:
>>> When calling debugfs functions, there is no need to ever check the
>>> return value. The function can work or not, but the code logic should
>>> never do something different based on this.
>>>
>>> Cc: Jason Baron <jbaron@akamai.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>> lib/dynamic_debug.c | 12 +++---------
>>> 1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>>> index 8a16c2d498e9..c60409138e13 100644
>>> --- a/lib/dynamic_debug.c
>>> +++ b/lib/dynamic_debug.c
>>> @@ -993,20 +993,14 @@ static __initdata int ddebug_init_success;
>>>
>>> static int __init dynamic_debug_init_debugfs(void)
>>> {
>>> - struct dentry *dir, *file;
>>> + struct dentry *dir;
>>>
>>> if (!ddebug_init_success)
>>> return -ENODEV;
>>>
>>> dir = debugfs_create_dir("dynamic_debug", NULL);
>>> - if (!dir)
>>> - return -ENOMEM;
>>> - file = debugfs_create_file("control", 0644, dir, NULL,
>>> - &ddebug_proc_fops);
>>> - if (!file) {
>>> - debugfs_remove(dir);
>>> - return -ENOMEM;
>>> - }
>>> + debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
>>> +
>>> return 0;
>>> }
>>>
>>>
>>
>> Looks like debugfs_create_dir() can return NULL,
>
> No it can not.
>
Ok, I looked at the wrong definition for failed_creating() :(
In that case:
Acked-by: Jason Baron <jbaron@akamai.com>
Thanks,
-Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions
2019-06-13 16:09 ` Jason Baron
@ 2019-06-13 17:41 ` Greg Kroah-Hartman
0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-13 17:41 UTC (permalink / raw)
To: Jason Baron; +Cc: linux-kernel
On Thu, Jun 13, 2019 at 12:09:00PM -0400, Jason Baron wrote:
>
>
> On 6/13/19 11:59 AM, Greg Kroah-Hartman wrote:
> > On Thu, Jun 13, 2019 at 10:33:23AM -0400, Jason Baron wrote:
> >> On 6/12/19 11:35 AM, Greg Kroah-Hartman wrote:
> >>> When calling debugfs functions, there is no need to ever check the
> >>> return value. The function can work or not, but the code logic should
> >>> never do something different based on this.
> >>>
> >>> Cc: Jason Baron <jbaron@akamai.com>
> >>> Cc: linux-kernel@vger.kernel.org
> >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >>> ---
> >>> lib/dynamic_debug.c | 12 +++---------
> >>> 1 file changed, 3 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> >>> index 8a16c2d498e9..c60409138e13 100644
> >>> --- a/lib/dynamic_debug.c
> >>> +++ b/lib/dynamic_debug.c
> >>> @@ -993,20 +993,14 @@ static __initdata int ddebug_init_success;
> >>>
> >>> static int __init dynamic_debug_init_debugfs(void)
> >>> {
> >>> - struct dentry *dir, *file;
> >>> + struct dentry *dir;
> >>>
> >>> if (!ddebug_init_success)
> >>> return -ENODEV;
> >>>
> >>> dir = debugfs_create_dir("dynamic_debug", NULL);
> >>> - if (!dir)
> >>> - return -ENOMEM;
> >>> - file = debugfs_create_file("control", 0644, dir, NULL,
> >>> - &ddebug_proc_fops);
> >>> - if (!file) {
> >>> - debugfs_remove(dir);
> >>> - return -ENOMEM;
> >>> - }
> >>> + debugfs_create_file("control", 0644, dir, NULL, &ddebug_proc_fops);
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>>
> >>
> >> Looks like debugfs_create_dir() can return NULL,
> >
> > No it can not.
> >
>
> Ok, I looked at the wrong definition for failed_creating() :(
Yeah, tracefs should also probably change, it makes users of their api
easier...
> In that case:
>
> Acked-by: Jason Baron <jbaron@akamai.com>
Wonderful, thanks for the review.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-13 17:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 15:35 [PATCH] lib: dynamic_debug: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-06-13 14:33 ` Jason Baron
2019-06-13 15:59 ` Greg Kroah-Hartman
2019-06-13 16:09 ` Jason Baron
2019-06-13 17:41 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.