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

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

[Yan Hong]

Pass NULL instead of empty string to debugfs_print_regs32() when
prefix is not used, according to the intention of the code.

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 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..3915cc9 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/3] debugfs: code clean and refactor

[Yan Hong]

debug_fill_super():
	return directly if failed to allocate debugfs_fs_info.

__create_file():
	remove redundant initialization.

__debugfs_remove():
	remove redundant check.
	debugfs_positive() already checked dentry->inode is not NULL.

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

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index b607d92..58eadc1 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)
@@ -323,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)) {
@@ -466,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

Re: [PATCH 2/3] debugfs: code clean and refactor

[Greg KH]

On Sat, Oct 27, 2012 at 01:55:43AM +0800, Yan Hong wrote:
> debug_fill_super():
> 	return directly if failed to allocate debugfs_fs_info.
> 
> __create_file():
> 	remove redundant initialization.
> 
> __debugfs_remove():
> 	remove redundant check.
> 	debugfs_positive() already checked dentry->inode is not NULL.

You did 3 different things, so that should be at least 3 different
patches.

thanks,

greg k-h

[PATCH 3/3] debugfs: break long lines

[Yan Hong]

Signed-off-by: Yan Hong <clouds.yan@gmail.com>
---
 fs/debugfs/file.c  |   42 ++++++++++++++++++++++++++++--------------
 fs/debugfs/inode.c |   10 ++++++----
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 3915cc9..b37eee1 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -95,10 +95,12 @@ struct dentry *debugfs_create_u8(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u8_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								&fops_u8_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u8_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								&fops_u8_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_u8);
 }
@@ -147,10 +149,12 @@ struct dentry *debugfs_create_u16(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u16_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								&fops_u16_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u16_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								&fops_u16_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_u16);
 }
@@ -199,10 +203,12 @@ struct dentry *debugfs_create_u32(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u32_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_u32_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u32_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_u32_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_u32);
 }
@@ -252,10 +258,12 @@ struct dentry *debugfs_create_u64(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u64_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_u64_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_u64_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_u64_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_u64);
 }
@@ -298,10 +306,12 @@ struct dentry *debugfs_create_x8(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x8_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x8_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x8_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x8_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_x8);
 }
@@ -322,10 +332,12 @@ struct dentry *debugfs_create_x16(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x16_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x16_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x16_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x16_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_x16);
 }
@@ -346,10 +358,12 @@ struct dentry *debugfs_create_x32(const char *name, umode_t mode,
 {
 	/* if there are no write bits set, make read only */
 	if (!(mode & S_IWUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x32_ro);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x32_ro);
 	/* if there are no read bits set, make write only */
 	if (!(mode & S_IRUGO))
-		return debugfs_create_file(name, mode, parent, value, &fops_x32_wo);
+		return debugfs_create_file(name, mode, parent, value,
+								 &fops_x32_wo);
 
 	return debugfs_create_file(name, mode, parent, value, &fops_x32);
 }
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 58eadc1..edbaf37 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -34,8 +34,9 @@ static struct vfsmount *debugfs_mount;
 static int debugfs_mount_count;
 static bool debugfs_registered;
 
-static struct inode *debugfs_get_inode(struct super_block *sb, umode_t mode, dev_t dev,
-				       void *data, const struct file_operations *fops)
+static struct inode *debugfs_get_inode(struct super_block *sb, umode_t mode,
+				dev_t dev,  void *data,
+				const struct file_operations *fops)
 
 {
 	struct inode *inode = new_inode(sb);
@@ -110,8 +111,9 @@ static int debugfs_link(struct inode *dir, struct dentry *dentry, umode_t mode,
 	return debugfs_mknod(dir, dentry, mode, 0, data, NULL);
 }
 
-static int debugfs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
-			  void *data, const struct file_operations *fops)
+static int debugfs_create(struct inode *dir, struct dentry *dentry,
+					umode_t mode, void *data,
+					const struct file_operations *fops)
 {
 	int res;
 
-- 
1.7.9.5

Re: [PATCH 3/3] debugfs: break long lines

[Greg KH]

On Sat, Oct 27, 2012 at 01:55:44AM +0800, Yan Hong wrote:
> 
> Signed-off-by: Yan Hong <clouds.yan@gmail.com>
> ---
>  fs/debugfs/file.c  |   42 ++++++++++++++++++++++++++++--------------
>  fs/debugfs/inode.c |   10 ++++++----
>  2 files changed, 34 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 3915cc9..b37eee1 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -95,10 +95,12 @@ struct dentry *debugfs_create_u8(const char *name, umode_t mode,
>  {
>  	/* if there are no write bits set, make read only */
>  	if (!(mode & S_IWUGO))
> -		return debugfs_create_file(name, mode, parent, value, &fops_u8_ro);
> +		return debugfs_create_file(name, mode, parent, value,
> +								&fops_u8_ro);

If you are going to do this (and hint, I really don't want you to), at
least make it look pretty:
		return debugfs_create_file(name, mode, parent, value,
					   &fops_u8_ro);

Other than that, I don't want to take this patch, sorry, it looks good
enough as-is.

thanks,

greg k-h

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

[Greg KH]

On Sat, Oct 27, 2012 at 01:55:42AM +0800, Yan Hong wrote:
> Pass NULL instead of empty string to debugfs_print_regs32() when
> prefix is not used, according to the intention of the code.

Is this solving a problem?  Is it wrong as-is?  Is it causing an issue
today?

thanks,

greg k-h

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

[yan yan]

2012/10/27 Greg KH <gregkh@linuxfoundation.org>:
> On Sat, Oct 27, 2012 at 01:55:42AM +0800, Yan Hong wrote:
>> Pass NULL instead of empty string to debugfs_print_regs32() when
>> prefix is not used, according to the intention of the code.
>
> Is this solving a problem?  Is it wrong as-is?  Is it causing an issue
> today?
>
> thanks,
>
> greg k-h

Hmmm ... this function is rarely used, and maybe over-designed. The
'prefix' feature can be actually removed -- nobody uses it.

As to the empty  string, it looks like a typo to me.

Thanks

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

[Greg KH]

On Sat, Oct 27, 2012 at 02:34:02AM +0800, yan yan wrote:
> 2012/10/27 Greg KH <gregkh@linuxfoundation.org>:
> > On Sat, Oct 27, 2012 at 01:55:42AM +0800, Yan Hong wrote:
> >> Pass NULL instead of empty string to debugfs_print_regs32() when
> >> prefix is not used, according to the intention of the code.
> >
> > Is this solving a problem?  Is it wrong as-is?  Is it causing an issue
> > today?
> >
> > thanks,
> >
> > greg k-h
> 
> Hmmm ... this function is rarely used, and maybe over-designed. The
> 'prefix' feature can be actually removed -- nobody uses it.

Then just remove it :)

thanks,

greg k-h