[PATCH 1/5] debugfs: pass NULL as the last parameter of debugfs_print_regs32()

[PATCH 1/5] debugfs: pass NULL as the last parameter of debugfs_print_regs32()

[Yan Hong]

If 'prefix' is not used, pass NULL instead of empty string as the
last parameter of debugfs_print_regs32().

Cc: Alessandro Rubini <rubini@gnudd.com>

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---

This function is only used (twice) by the author of it, and the
'prefix' feature is never used. But it's a well-documented debugfs
API. It's added one year ago and appeared in many documents. I still
prefer to let this API stay unchanged and treat the empty string as
typo.

 drivers/usb/dwc3/debugfs.c |    2 +-
 fs/debugfs/file.c          |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/debugfs.c b/drivers/usb/dwc3/debugfs.c
index d4a30f1..6557272 100644
--- a/drivers/usb/dwc3/debugfs.c
+++ b/drivers/usb/dwc3/debugfs.c
@@ -382,7 +382,7 @@ static int dwc3_regdump_show(struct seq_file *s, void *unused)
 
 	seq_printf(s, "DesignWare USB3 Core Register Dump\n");
 	debugfs_print_regs32(s, dwc3_regs, ARRAY_SIZE(dwc3_regs),
-			     dwc->regs, "");
+			     dwc->regs, NULL);
 	return 0;
 }
 
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index c5ca6ae..6bdd450 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -668,7 +668,8 @@ static int debugfs_show_regset32(struct seq_file *s, void *data)
 {
 	struct debugfs_regset32 *regset = s->private;
 
-	debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
+	debugfs_print_regs32(s, regset->regs, regset->nregs,
+			regset->base, NULL);
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 2/5] debugfs: return directly if failed to allocate memory in debug_fill_super()

[Yan Hong]

If kzalloc failed, memory is not allocated, sb->s_fs_info is NULL.
No need to go through clean up code, return -ENOMEM directly.

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 fs/debugfs/inode.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b607d92..11f6514 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -262,10 +262,8 @@ static int debug_fill_super(struct super_block *sb, void *data, int silent)
 
 	fsi = kzalloc(sizeof(struct debugfs_fs_info), GFP_KERNEL);
 	sb->s_fs_info = fsi;
-	if (!fsi) {
-		err = -ENOMEM;
-		goto fail;
-	}
+	if (!fsi)
+		return -ENOMEM;
 
 	err = debugfs_parse_options(data, &fsi->mount_opts);
 	if (err)
-- 
1.7.9.5

[PATCH 3/5] debugfs: remove redundant initialization in __create_file()

[Yan Hong]

'dentry' has been set to NULL at the beginning of the function.

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 fs/debugfs/inode.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 11f6514..12f1282 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -321,7 +321,6 @@ static struct dentry *__create_file(const char *name, umode_t mode,
 	if (!parent)
 		parent = debugfs_mount->mnt_root;
 
-	dentry = NULL;
 	mutex_lock(&parent->d_inode->i_mutex);
 	dentry = lookup_one_len(name, parent, strlen(name));
 	if (!IS_ERR(dentry)) {
-- 
1.7.9.5

[PATCH 4/5] debugfs: remove redundant check in __debugfs_remove()

[Yan Hong]

debugfs_positive() already checked dentry->d_inode is
not NUUL, no need to check it again.

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 fs/debugfs/inode.c |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 12f1282..58eadc1 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -463,23 +463,21 @@ static int __debugfs_remove(struct dentry *dentry, struct dentry *parent)
 	int ret = 0;
 
 	if (debugfs_positive(dentry)) {
-		if (dentry->d_inode) {
-			dget(dentry);
-			switch (dentry->d_inode->i_mode & S_IFMT) {
-			case S_IFDIR:
-				ret = simple_rmdir(parent->d_inode, dentry);
-				break;
-			case S_IFLNK:
-				kfree(dentry->d_inode->i_private);
-				/* fall through */
-			default:
-				simple_unlink(parent->d_inode, dentry);
-				break;
-			}
-			if (!ret)
-				d_delete(dentry);
-			dput(dentry);
+		dget(dentry);
+		switch (dentry->d_inode->i_mode & S_IFMT) {
+		case S_IFDIR:
+			ret = simple_rmdir(parent->d_inode, dentry);
+			break;
+		case S_IFLNK:
+			kfree(dentry->d_inode->i_private);
+			/* fall through */
+		default:
+			simple_unlink(parent->d_inode, dentry);
+			break;
 		}
+		if (!ret)
+			d_delete(dentry);
+		dput(dentry);
 	}
 	return ret;
 }
-- 
1.7.9.5

[PATCH 5/5] debugfs: remove goto in debugfs_remount()

[Yan Hong]

Simple code clean to remove goto.

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 fs/debugfs/inode.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 58eadc1..c0b06a3 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -221,12 +221,11 @@ static int debugfs_remount(struct super_block *sb, int *flags, char *data)
 
 	err = debugfs_parse_options(data, &fsi->mount_opts);
 	if (err)
-		goto fail;
+		return err;
 
 	debugfs_apply_options(sb);
 
-fail:
-	return err;
+	return 0;
 }
 
 static int debugfs_show_options(struct seq_file *m, struct dentry *root)
-- 
1.7.9.5

Re: [PATCH 5/5] debugfs: remove goto in debugfs_remount()

[Greg KH]

On Sat, Oct 27, 2012 at 04:05:29PM +0800, Yan Hong wrote:
> Simple code clean to remove goto.

There is nothing wrong with gotos on error paths, so this one should
stay.

greg k-h

Re: [PATCH 5/5] debugfs: remove goto in debugfs_remount()

[Yan Hong]

2012/10/27 Greg KH <gregkh@linuxfoundation.org>:
> On Sat, Oct 27, 2012 at 04:05:29PM +0800, Yan Hong wrote:
>> Simple code clean to remove goto.
>
> There is nothing wrong with gotos on error paths, so this one should
> stay.
>
> greg k-h

I thought we use goto on error path because there is no other elegant
ways to do the things, but it sounds like goto is still welcome and preferable
when we are not in this case. Anyway, this is a simple clean, I do not insist
on it.

Thanks

Re: [PATCH 1/5] debugfs: pass NULL as the last parameter of debugfs_print_regs32()

[Greg KH]

On Sat, Oct 27, 2012 at 04:05:25PM +0800, Yan Hong wrote:
> If 'prefix' is not used, pass NULL instead of empty string as the
> last parameter of debugfs_print_regs32().
> 
> Cc: Alessandro Rubini <rubini@gnudd.com>
> 
> Signed-off-by: Yan Hong <clouds.yan@gmail.com>
> ---
> 
> This function is only used (twice) by the author of it, and the
> 'prefix' feature is never used. But it's a well-documented debugfs
> API. It's added one year ago and appeared in many documents. I still
> prefer to let this API stay unchanged and treat the empty string as
> typo.

What "documents" did this appear in?  If the parameter isn't being used,
please, just delete it.

greg k-h

Re: [PATCH 1/5] debugfs: pass NULL as the last parameter of debugfs_print_regs32()

[Alessandro Rubini]

>> This function is only used (twice) by the author of it, and the
>> 'prefix' feature is never used.

I introduced it to shrink some internal code that had a lot of
repetition in the debugfs files implementation. In that context I use
the prefix string.  Some of the code had later been submitted, but not
yet all of it.  Later work by Davide Ciminaghi moved our MFD user to
the regmap interface, according to maintainer's suggestion (which
however means loosing register names from the debugfs dump).

> If the parameter isn't being used, please, just delete it.

It is not currently used by upstream callers, but the function itself
uses it as documented (Documentation/...).  I wouldn't remove the
feature and introduce an incompatibility just for this.

/alessandro

Re: [PATCH 1/5] debugfs: pass NULL as the last parameter of debugfs_print_regs32()

[Greg KH]

On Sat, Oct 27, 2012 at 05:45:49PM +0200, Alessandro Rubini wrote:
> >> This function is only used (twice) by the author of it, and the
> >> 'prefix' feature is never used.
> 
> I introduced it to shrink some internal code that had a lot of
> repetition in the debugfs files implementation. In that context I use
> the prefix string.  Some of the code had later been submitted, but not
> yet all of it.  Later work by Davide Ciminaghi moved our MFD user to
> the regmap interface, according to maintainer's suggestion (which
> however means loosing register names from the debugfs dump).

What is the status of the rest of that code getting accepted?

> > If the parameter isn't being used, please, just delete it.
> 
> It is not currently used by upstream callers, but the function itself
> uses it as documented (Documentation/...).  I wouldn't remove the
> feature and introduce an incompatibility just for this.

But if no one is using it, we can fix the documentation, and just drop
it.

thanks,

greg k-h