* Re: [PATCH] block: aoe: no need to check return value of debugfs_create functions
2019-01-22 15:21 [PATCH] block: aoe: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-01-22 23:29 ` Omar Sandoval
2019-01-23 0:31 ` Ed Cashin
2019-01-23 6:41 ` Greg Kroah-Hartman
2019-06-03 14:25 ` [PATCH v2] " Greg Kroah-Hartman
2019-06-03 19:47 ` [PATCH v3] " Greg Kroah-Hartman
2 siblings, 2 replies; 8+ messages in thread
From: Omar Sandoval @ 2019-01-22 23:29 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, Ed L. Cashin, Jens Axboe, linux-block
On Tue, Jan 22, 2019 at 04:21:04PM +0100, 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: "Ed L. Cashin" <ed.cashin@acm.org>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: linux-block@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/block/aoe/aoeblk.c | 13 +------------
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
> index e2c6aae2d636..b602646bfa04 100644
> --- a/drivers/block/aoe/aoeblk.c
> +++ b/drivers/block/aoe/aoeblk.c
> @@ -207,14 +207,7 @@ aoedisk_add_debugfs(struct aoedev *d)
> else
> p++;
> BUG_ON(*p == '\0');
> - entry = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
> - &aoe_debugfs_fops);
> - if (IS_ERR_OR_NULL(entry)) {
> - pr_info("aoe: cannot create debugfs file for %s\n",
> - d->gd->disk_name);
> - return;
> - }
> - BUG_ON(d->debugfs);
> + debugfs_create_file(p, 0444, aoe_debugfs_dir, d, &aoe_debugfs_fops);
> d->debugfs = entry;
Now entry is uninitialized here when we assign it to d->debugfs.
> }
> void
> @@ -472,10 +465,6 @@ aoeblk_init(void)
> if (buf_pool_cache == NULL)
> return -ENOMEM;
> aoe_debugfs_dir = debugfs_create_dir("aoe", NULL);
> - if (IS_ERR_OR_NULL(aoe_debugfs_dir)) {
> - pr_info("aoe: cannot create debugfs directory\n");
> - aoe_debugfs_dir = NULL;
> - }
> return 0;
> }
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] block: aoe: no need to check return value of debugfs_create functions
2019-01-22 23:29 ` Omar Sandoval
@ 2019-01-23 0:31 ` Ed Cashin
2019-01-23 6:41 ` Greg Kroah-Hartman
1 sibling, 0 replies; 8+ messages in thread
From: Ed Cashin @ 2019-01-23 0:31 UTC (permalink / raw)
To: Omar Sandoval
Cc: Greg Kroah-Hartman, linux-kernel, Ed L. Cashin, Jens Axboe,
linux-block
On Tue, Jan 22, 2019 at 6:29 PM Omar Sandoval <osandov@osandov.com> wrote:
...
> Now entry is uninitialized here when we assign it to d->debugfs.
Thanks for noticing that.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] block: aoe: no need to check return value of debugfs_create functions
2019-01-22 23:29 ` Omar Sandoval
2019-01-23 0:31 ` Ed Cashin
@ 2019-01-23 6:41 ` Greg Kroah-Hartman
1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-23 6:41 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-kernel, Ed L. Cashin, Jens Axboe, linux-block
On Tue, Jan 22, 2019 at 03:29:33PM -0800, Omar Sandoval wrote:
> On Tue, Jan 22, 2019 at 04:21:04PM +0100, 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: "Ed L. Cashin" <ed.cashin@acm.org>
> > Cc: Jens Axboe <axboe@kernel.dk>
> > Cc: linux-block@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > drivers/block/aoe/aoeblk.c | 13 +------------
> > 1 file changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
> > index e2c6aae2d636..b602646bfa04 100644
> > --- a/drivers/block/aoe/aoeblk.c
> > +++ b/drivers/block/aoe/aoeblk.c
> > @@ -207,14 +207,7 @@ aoedisk_add_debugfs(struct aoedev *d)
> > else
> > p++;
> > BUG_ON(*p == '\0');
> > - entry = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
> > - &aoe_debugfs_fops);
> > - if (IS_ERR_OR_NULL(entry)) {
> > - pr_info("aoe: cannot create debugfs file for %s\n",
> > - d->gd->disk_name);
> > - return;
> > - }
> > - BUG_ON(d->debugfs);
> > + debugfs_create_file(p, 0444, aoe_debugfs_dir, d, &aoe_debugfs_fops);
> > d->debugfs = entry;
>
> Now entry is uninitialized here when we assign it to d->debugfs.
Ah, good catch, I'll go fix this up and post a v2, sorry about that.
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] block: aoe: no need to check return value of debugfs_create functions
2019-01-22 15:21 [PATCH] block: aoe: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-01-22 23:29 ` Omar Sandoval
@ 2019-06-03 14:25 ` Greg Kroah-Hartman
2019-06-03 19:47 ` [PATCH v3] " Greg Kroah-Hartman
2 siblings, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-03 14:25 UTC (permalink / raw)
To: linux-kernel; +Cc: Ed L. Cashin, Jens Axboe, linux-block
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: "Ed L. Cashin" <ed.cashin@acm.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org
Cc: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: fix uninitialized entry issue found by Omar Sandoval
drivers/block/aoe/aoeblk.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index e2c6aae2d636..bd19f8af950b 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -196,7 +196,6 @@ static const struct file_operations aoe_debugfs_fops = {
static void
aoedisk_add_debugfs(struct aoedev *d)
{
- struct dentry *entry;
char *p;
if (aoe_debugfs_dir == NULL)
@@ -207,15 +206,8 @@ aoedisk_add_debugfs(struct aoedev *d)
else
p++;
BUG_ON(*p == '\0');
- entry = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
- &aoe_debugfs_fops);
- if (IS_ERR_OR_NULL(entry)) {
- pr_info("aoe: cannot create debugfs file for %s\n",
- d->gd->disk_name);
- return;
- }
- BUG_ON(d->debugfs);
- d->debugfs = entry;
+ d->debugfs = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
+ &aoe_debugfs_fops);
}
void
aoedisk_rm_debugfs(struct aoedev *d)
@@ -472,10 +464,6 @@ aoeblk_init(void)
if (buf_pool_cache == NULL)
return -ENOMEM;
aoe_debugfs_dir = debugfs_create_dir("aoe", NULL);
- if (IS_ERR_OR_NULL(aoe_debugfs_dir)) {
- pr_info("aoe: cannot create debugfs directory\n");
- aoe_debugfs_dir = NULL;
- }
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3] block: aoe: no need to check return value of debugfs_create functions
2019-01-22 15:21 [PATCH] block: aoe: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-01-22 23:29 ` Omar Sandoval
2019-06-03 14:25 ` [PATCH v2] " Greg Kroah-Hartman
@ 2019-06-03 19:47 ` Greg Kroah-Hartman
2019-06-04 18:09 ` Justin Sanders
2019-06-04 19:38 ` Jens Axboe
2 siblings, 2 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-03 19:47 UTC (permalink / raw)
To: Justin Sanders, linux-kernel; +Cc: Ed L. Cashin, Jens Axboe, linux-block
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: "Ed L. Cashin" <ed.cashin@acm.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org
Cc: Omar Sandoval <osandov@osandov.com>
Cc: Justin Sanders <justin@coraid.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: Justin is now the maintainer of this driver, properly send it to
him.
v2: fix uninitialized entry issue found by Omar Sandoval
drivers/block/aoe/aoeblk.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
index e2c6aae2d636..bd19f8af950b 100644
--- a/drivers/block/aoe/aoeblk.c
+++ b/drivers/block/aoe/aoeblk.c
@@ -196,7 +196,6 @@ static const struct file_operations aoe_debugfs_fops = {
static void
aoedisk_add_debugfs(struct aoedev *d)
{
- struct dentry *entry;
char *p;
if (aoe_debugfs_dir == NULL)
@@ -207,15 +206,8 @@ aoedisk_add_debugfs(struct aoedev *d)
else
p++;
BUG_ON(*p == '\0');
- entry = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
- &aoe_debugfs_fops);
- if (IS_ERR_OR_NULL(entry)) {
- pr_info("aoe: cannot create debugfs file for %s\n",
- d->gd->disk_name);
- return;
- }
- BUG_ON(d->debugfs);
- d->debugfs = entry;
+ d->debugfs = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
+ &aoe_debugfs_fops);
}
void
aoedisk_rm_debugfs(struct aoedev *d)
@@ -472,10 +464,6 @@ aoeblk_init(void)
if (buf_pool_cache == NULL)
return -ENOMEM;
aoe_debugfs_dir = debugfs_create_dir("aoe", NULL);
- if (IS_ERR_OR_NULL(aoe_debugfs_dir)) {
- pr_info("aoe: cannot create debugfs directory\n");
- aoe_debugfs_dir = NULL;
- }
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3] block: aoe: no need to check return value of debugfs_create functions
2019-06-03 19:47 ` [PATCH v3] " Greg Kroah-Hartman
@ 2019-06-04 18:09 ` Justin Sanders
2019-06-04 19:38 ` Jens Axboe
1 sibling, 0 replies; 8+ messages in thread
From: Justin Sanders @ 2019-06-04 18:09 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-kernel@vger.kernel.org, Ed L. Cashin, Jens Axboe,
linux-block@vger.kernel.org
Hi,
> On Jun 3, 2019, at 3:47 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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: "Ed L. Cashin" <ed.cashin@acm.org>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: linux-block@vger.kernel.org
> Cc: Omar Sandoval <osandov@osandov.com>
> Cc: Justin Sanders <justin@coraid.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> v3: Justin is now the maintainer of this driver, properly send it to
> him.
> v2: fix uninitialized entry issue found by Omar Sandoval
>
> drivers/block/aoe/aoeblk.c | 16 ++--------------
> 1 file changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/block/aoe/aoeblk.c b/drivers/block/aoe/aoeblk.c
> index e2c6aae2d636..bd19f8af950b 100644
> --- a/drivers/block/aoe/aoeblk.c
> +++ b/drivers/block/aoe/aoeblk.c
> @@ -196,7 +196,6 @@ static const struct file_operations aoe_debugfs_fops = {
> static void
> aoedisk_add_debugfs(struct aoedev *d)
> {
> - struct dentry *entry;
> char *p;
>
> if (aoe_debugfs_dir == NULL)
> @@ -207,15 +206,8 @@ aoedisk_add_debugfs(struct aoedev *d)
> else
> p++;
> BUG_ON(*p == '\0');
> - entry = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
> - &aoe_debugfs_fops);
> - if (IS_ERR_OR_NULL(entry)) {
> - pr_info("aoe: cannot create debugfs file for %s\n",
> - d->gd->disk_name);
> - return;
> - }
> - BUG_ON(d->debugfs);
> - d->debugfs = entry;
> + d->debugfs = debugfs_create_file(p, 0444, aoe_debugfs_dir, d,
> + &aoe_debugfs_fops);
> }
> void
> aoedisk_rm_debugfs(struct aoedev *d)
> @@ -472,10 +464,6 @@ aoeblk_init(void)
> if (buf_pool_cache == NULL)
> return -ENOMEM;
> aoe_debugfs_dir = debugfs_create_dir("aoe", NULL);
> - if (IS_ERR_OR_NULL(aoe_debugfs_dir)) {
> - pr_info("aoe: cannot create debugfs directory\n");
> - aoe_debugfs_dir = NULL;
> - }
> return 0;
> }
>
> --
> 2.21.0
>
>
This looks good to me, thanks.
Acked-by: Justin Sanders <justin@coraid.com>
Cheers,
Justin
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v3] block: aoe: no need to check return value of debugfs_create functions
2019-06-03 19:47 ` [PATCH v3] " Greg Kroah-Hartman
2019-06-04 18:09 ` Justin Sanders
@ 2019-06-04 19:38 ` Jens Axboe
1 sibling, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2019-06-04 19:38 UTC (permalink / raw)
To: Greg Kroah-Hartman, Justin Sanders, linux-kernel
Cc: Ed L. Cashin, linux-block
On 6/3/19 1:47 PM, 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.
Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread