[PATCH 0/2] UBIFS: fix error path in dbg_debugfs_init_fs

[PATCH 0/2] UBIFS: fix error path in dbg_debugfs_init_fs

[Artem Bityutskiy]

Hi Phil,

I was not completely happy about your fix and here is the fix I like
more. It consists of 2 patches - the first one is preparational and
the second one is the fix. Please, take a look.

Artem.

[PATCH 1/2] UBIFS: unify error path dbg_debugfs_init_fs

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

This is just a small clean-up patch which simlifies and unifies the
error path in the dbg_debugfs_init_fs(). We have common error path
for all failure cases in this function except of the very first
case. And this patch makes the first failure case use the same
error path as the other cases by using the 'fname' and 'dent'
variables.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/debug.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 68cc992..4abaf72 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2807,13 +2807,11 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)
 	struct ubifs_debug_info *d = c->dbg;
 
 	sprintf(d->dfs_dir_name, "ubi%d_%d", c->vi.ubi_num, c->vi.vol_id);
-	d->dfs_dir = debugfs_create_dir(d->dfs_dir_name, dfs_rootdir);
-	if (IS_ERR(d->dfs_dir)) {
-		err = PTR_ERR(d->dfs_dir);
-		ubifs_err("cannot create \"%s\" debugfs directory, error %d\n",
-			  d->dfs_dir_name, err);
+	fname = d->dfs_dir_name;
+	dent = debugfs_create_dir(fname, dfs_rootdir);
+	if (IS_ERR(dent))
 		goto out;
-	}
+	d->dfs_dir = dent;
 
 	fname = "dump_lprops";
 	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
@@ -2836,11 +2834,11 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)
 	return 0;
 
 out_remove:
+	debugfs_remove_recursive(d->dfs_dir);
+out:
 	err = PTR_ERR(dent);
 	ubifs_err("cannot create \"%s\" debugfs directory, error %d\n",
 		  fname, err);
-	debugfs_remove_recursive(d->dfs_dir);
-out:
 	return err;
 }
 
-- 
1.7.2.3

[PATCH 2/2] UBIFS: fix error path in dbg_debugfs_init_fs

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

The debug interface is substandard and on error returns either
NULL or an error code packed in the pointer. So using "IS_ERR"
for the pointers returned by debugfs function is incorrect.
Instead, we should use IS_ERR_OR_NULL.

This path is an improved vestion of the original patch from
Phil Carmody.

Reported-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 fs/ubifs/debug.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 4abaf72..c04efa8 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -2809,25 +2809,25 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)
 	sprintf(d->dfs_dir_name, "ubi%d_%d", c->vi.ubi_num, c->vi.vol_id);
 	fname = d->dfs_dir_name;
 	dent = debugfs_create_dir(fname, dfs_rootdir);
-	if (IS_ERR(dent))
+	if (IS_ERR_OR_NULL(dent))
 		goto out;
 	d->dfs_dir = dent;
 
 	fname = "dump_lprops";
 	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
-	if (IS_ERR(dent))
+	if (IS_ERR_OR_NULL(dent))
 		goto out_remove;
 	d->dfs_dump_lprops = dent;
 
 	fname = "dump_budg";
 	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
-	if (IS_ERR(dent))
+	if (IS_ERR_OR_NULL(dent))
 		goto out_remove;
 	d->dfs_dump_budg = dent;
 
 	fname = "dump_tnc";
 	dent = debugfs_create_file(fname, S_IWUSR, d->dfs_dir, c, &dfs_fops);
-	if (IS_ERR(dent))
+	if (IS_ERR_OR_NULL(dent))
 		goto out_remove;
 	d->dfs_dump_tnc = dent;
 
@@ -2836,7 +2836,7 @@ int dbg_debugfs_init_fs(struct ubifs_info *c)
 out_remove:
 	debugfs_remove_recursive(d->dfs_dir);
 out:
-	err = PTR_ERR(dent);
+	err = dent ? PTR_ERR(dent) : -ENODEV;
 	ubifs_err("cannot create \"%s\" debugfs directory, error %d\n",
 		  fname, err);
 	return err;
-- 
1.7.2.3

Re: [PATCH 0/2] UBIFS: fix error path in dbg_debugfs_init_fs

[Phil Carmody]

On 01/04/11 10:25 +0300, Artem Bityutskiy wrote:
> I was not completely happy about your fix and here is the fix I like
> more. It consists of 2 patches - the first one is preparational and
> the second one is the fix. Please, take a look.

The consolidation of the error paths certainly makes sense,
and indeed makes the fix much more clinical. Fully approve.

Acked-by: Phil Carmody <ext-phil.2.carmody@nokia.com>

Phil

Re: [PATCH 0/2] UBIFS: fix error path in dbg_debugfs_init_fs

[Artem Bityutskiy]

On Fri, 2011-04-01 at 13:00 +0300, Phil Carmody wrote:
> On 01/04/11 10:25 +0300, Artem Bityutskiy wrote:
> > I was not completely happy about your fix and here is the fix I like
> > more. It consists of 2 patches - the first one is preparational and
> > the second one is the fix. Please, take a look.
> 
> The consolidation of the error paths certainly makes sense,
> and indeed makes the fix much more clinical. Fully approve.
> 
> Acked-by: Phil Carmody <ext-phil.2.carmody@nokia.com>

OK, thanks, pushed to ubifs-2.6.git tree.

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)