Skip to contentSkip to conversation

debugfs: pass NULL as the last parameter of debugfs_print_regs32()

PATCH In review Yan Hong 2 participants 8 messages Oct 26, 2012

Subsystems (3)
USB Driver core DesignWare USB3 DRD IP driver
lore
Share & tools
lore

Yan Hong

author
#0 lore raw

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

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 <[email protected]>
---
 drivers/usb/dwc3/debugfs.c |    2 +-
 fs/debugfs/file.c          |    3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
drivers/usb/dwc3/debugfs.c +1 -1
index d4a30f1..6557272 100644
@@ -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;
 }
fs/debugfs/file.c +2 -1
index c5ca6ae..3915cc9 100644
@@ -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;
 }
Signature · 1 line
1.7.9.5
author
#1 lore raw

PATCH 2/3 debugfs: code clean and refactor

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 <[email protected]>
---
 fs/debugfs/inode.c |   37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)
fs/debugfs/inode.c +16 -21
index b607d92..58eadc1 100644
@@ -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;
 }
Signature · 1 line
1.7.9.5
maintainer
#2 lore raw

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

Quoted 9 lines · 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.

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

thanks,

greg k-h

author
#3 lore raw

PATCH 3/3 debugfs: break long lines

In reply to Yan Hong
Signed-off-by Yan Hong <[email protected]>
---
 fs/debugfs/file.c  |   42 ++++++++++++++++++++++++++++--------------
 fs/debugfs/inode.c |   10 ++++++----
 2 files changed, 34 insertions(+), 18 deletions(-)
fs/debugfs/file.c +28 -14
index 3915cc9..b37eee1 100644
@@ -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);
 }
fs/debugfs/inode.c +6 -4
index 58eadc1..edbaf37 100644
@@ -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;
Signature · 1 line
1.7.9.5
maintainer
#4 lore raw

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

Quoted 12 lines · Yan Hong
Signed-off-by Yan Hong <[email protected]>
---
 fs/debugfs/file.c  |   42 ++++++++++++++++++++++++++++--------------
 fs/debugfs/inode.c |   10 ++++++----
 2 files changed, 34 insertions(+), 18 deletions(-)
fs/debugfs/file.c +0 -0
index 3915cc9..b37eee1 100644

fs/debugfs/file.c struct dentry *debugfs_create_u8(const char *name, umode_t mode, · line 99

Quoted 6 lines
fs/debugfs/file.c +2 -1
@@ -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

maintainer
#5 lore raw
In reply to Yan Hong
Quoted 2 lines · Yan Hong

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

author
#6 lore raw

2012/10/27 Greg KH <[email protected]>:

Quoted 10 lines · Yan Hong
Quoted 2 lines

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

maintainer
#7 lore raw
Quoted 8 lines · yan yan

2012/10/27 Greg KH <[email protected]>:

Quoted 7 lines · Yan Hong
Quoted 2 lines

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?

Quoted 6 lines
Quoted 3 lines

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