debugfs: pass NULL as the last parameter of debugfs_print_regs32()
Subsystems (3)
PATCH 1/5 debugfs: pass NULL as the last parameter of debugfs_print_regs32()
If 'prefix' is not used, pass NULL instead of empty string as the last parameter of debugfs_print_regs32().
---
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(-)
drivers/usb/dwc3/debugfs.c +1 -1
@@ -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
@@ -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
PATCH 2/5 debugfs: return directly if failed to allocate memory in debug_fill_super()
If kzalloc failed, memory is not allocated, sb->s_fs_info is NULL. No need to go through clean up code, return -ENOMEM directly.
--- fs/debugfs/inode.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
fs/debugfs/inode.c +2 -4
@@ -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)
Signature · 1 line
1.7.9.5
PATCH 3/5 debugfs: remove redundant initialization in __create_file()
In reply to Yan Hong'dentry' has been set to NULL at the beginning of the function.
--- fs/debugfs/inode.c | 1 - 1 file changed, 1 deletion(-)
fs/debugfs/inode.c +0 -1
@@ -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)) {
Signature · 1 line
1.7.9.5
PATCH 4/5 debugfs: remove redundant check in __debugfs_remove()
In reply to Yan Hongdebugfs_positive() already checked dentry->d_inode is not NUUL, no need to check it again.
--- fs/debugfs/inode.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-)
fs/debugfs/inode.c +14 -16
@@ -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; }
Signature · 1 line
1.7.9.5
PATCH 5/5 debugfs: remove goto in debugfs_remount()
In reply to Yan HongSimple code clean to remove goto.
--- fs/debugfs/inode.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
fs/debugfs/inode.c +2 -3
@@ -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)
Signature · 1 line
1.7.9.5
Greg KH
maintainerRe: [PATCH 5/5] debugfs: remove goto in debugfs_remount()
Quoted 1 line · Yan Hong
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()
2012/10/27 Greg KH <[email protected]>:
Quoted 7 lines · Yan Hong
Quoted 1 line
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
Greg KH
maintainerQuoted 5 lines · Yan Hong
If 'prefix' is not used, pass NULL instead of empty string as the last parameter of
debugfs_print_regs32().Cc Alessandro Rubini <[email protected]>
Quoted 8 lines
Signed-off-by Yan Hong <[email protected]>---
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
Quoted 2 lines
Quoted 2 lines
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).
Quoted 1 line
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
Greg KH
maintainerQuoted 9 lines · Alessandro Rubini
Quoted 2 lines
Quoted 2 lines
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?
Quoted 5 lines
Quoted 1 line
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