[PATCH RESEND v2] char: misc:use DEFINE_PROC_SHOW_ATTRIBUTE micro to simplify misc proc_fops

[PATCH RESEND v2] char: misc:use DEFINE_PROC_SHOW_ATTRIBUTE micro to simplify misc proc_fops

[Junwen Wu]

From: wujunwen <wudaemon@163.com>

DEFINE_PROC_SHOW_ATTRIBUTE is used to simply seq_file flow ,so
we can use it to simplify misc proc_fops.

Signed-off-by: wujunwen <wudaemon@163.com>
---
V1 -> V2: Fix compile error with no "CONFIG_PROC_FS"

 drivers/char/misc.c | 41 ++++++++++++++---------------------------
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index ca5141ed5ef3..076b7f08aa7a 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -64,40 +64,23 @@ static DEFINE_MUTEX(misc_mtx);
 static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
 
 #ifdef CONFIG_PROC_FS
-static void *misc_seq_start(struct seq_file *seq, loff_t *pos)
-{
-	mutex_lock(&misc_mtx);
-	return seq_list_start(&misc_list, *pos);
-}
 
-static void *misc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
+static int misc_show(struct seq_file *seq, void *v)
 {
-	return seq_list_next(v, &misc_list, pos);
-}
+	const struct miscdevice *p;
 
-static void misc_seq_stop(struct seq_file *seq, void *v)
-{
+	mutex_lock(&misc_mtx);
+	list_for_each_entry(p, &misc_list, list) {
+		seq_printf(seq, "%3i %s\n", p->minor, p->name ? p->name : "");
+	}
 	mutex_unlock(&misc_mtx);
-}
-
-static int misc_seq_show(struct seq_file *seq, void *v)
-{
-	const struct miscdevice *p = list_entry(v, struct miscdevice, list);
-
-	seq_printf(seq, "%3i %s\n", p->minor, p->name ? p->name : "");
 	return 0;
 }
 
-
-static const struct seq_operations misc_seq_ops = {
-	.start = misc_seq_start,
-	.next  = misc_seq_next,
-	.stop  = misc_seq_stop,
-	.show  = misc_seq_show,
-};
+DEFINE_PROC_SHOW_ATTRIBUTE(misc);
 #endif
 
-static int misc_open(struct inode *inode, struct file *file)
+static int misc_fops_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
 	struct miscdevice *c;
@@ -148,7 +131,7 @@ static struct class *misc_class;
 
 static const struct file_operations misc_fops = {
 	.owner		= THIS_MODULE,
-	.open		= misc_open,
+	.open		= misc_fops_open,
 	.llseek		= noop_llseek,
 };
 
@@ -266,9 +249,11 @@ static char *misc_devnode(struct device *dev, umode_t *mode)
 static int __init misc_init(void)
 {
 	int err;
+#ifdef CONFIG_PROC_FS
 	struct proc_dir_entry *ret;
 
-	ret = proc_create_seq("misc", 0, NULL, &misc_seq_ops);
+	ret = proc_create("misc", 0, NULL, &misc_proc_ops);
+#endif
 	misc_class = class_create(THIS_MODULE, "misc");
 	err = PTR_ERR(misc_class);
 	if (IS_ERR(misc_class))
@@ -284,8 +269,10 @@ static int __init misc_init(void)
 	pr_err("unable to get major %d for misc devices\n", MISC_MAJOR);
 	class_destroy(misc_class);
 fail_remove:
+#ifdef CONFIG_PROC_FS
 	if (ret)
 		remove_proc_entry("misc", NULL);
+#endif
 	return err;
 }
 subsys_initcall(misc_init);
-- 
2.25.1

Re: [PATCH RESEND v2] char: misc:use DEFINE_PROC_SHOW_ATTRIBUTE micro to simplify misc proc_fops

[Greg KH]

On Sun, Apr 10, 2022 at 03:16:27PM +0000, Junwen Wu wrote:
> From: wujunwen <wudaemon@163.com>
> 
> DEFINE_PROC_SHOW_ATTRIBUTE is used to simply seq_file flow ,so
> we can use it to simplify misc proc_fops.
> 
> Signed-off-by: wujunwen <wudaemon@163.com>
> ---
> V1 -> V2: Fix compile error with no "CONFIG_PROC_FS"
> 
>  drivers/char/misc.c | 41 ++++++++++++++---------------------------
>  1 file changed, 14 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/char/misc.c b/drivers/char/misc.c
> index ca5141ed5ef3..076b7f08aa7a 100644
> --- a/drivers/char/misc.c
> +++ b/drivers/char/misc.c
> @@ -64,40 +64,23 @@ static DEFINE_MUTEX(misc_mtx);
>  static DECLARE_BITMAP(misc_minors, DYNAMIC_MINORS);
>  
>  #ifdef CONFIG_PROC_FS
> -static void *misc_seq_start(struct seq_file *seq, loff_t *pos)
> -{
> -	mutex_lock(&misc_mtx);
> -	return seq_list_start(&misc_list, *pos);
> -}
>  
> -static void *misc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> +static int misc_show(struct seq_file *seq, void *v)
>  {
> -	return seq_list_next(v, &misc_list, pos);
> -}
> +	const struct miscdevice *p;
>  
> -static void misc_seq_stop(struct seq_file *seq, void *v)
> -{
> +	mutex_lock(&misc_mtx);
> +	list_for_each_entry(p, &misc_list, list) {
> +		seq_printf(seq, "%3i %s\n", p->minor, p->name ? p->name : "");
> +	}
>  	mutex_unlock(&misc_mtx);
> -}
> -
> -static int misc_seq_show(struct seq_file *seq, void *v)
> -{
> -	const struct miscdevice *p = list_entry(v, struct miscdevice, list);
> -
> -	seq_printf(seq, "%3i %s\n", p->minor, p->name ? p->name : "");
>  	return 0;
>  }
>  
> -
> -static const struct seq_operations misc_seq_ops = {
> -	.start = misc_seq_start,
> -	.next  = misc_seq_next,
> -	.stop  = misc_seq_stop,
> -	.show  = misc_seq_show,
> -};
> +DEFINE_PROC_SHOW_ATTRIBUTE(misc);
>  #endif
>  
> -static int misc_open(struct inode *inode, struct file *file)
> +static int misc_fops_open(struct inode *inode, struct file *file)
>  {
>  	int minor = iminor(inode);
>  	struct miscdevice *c;
> @@ -148,7 +131,7 @@ static struct class *misc_class;
>  
>  static const struct file_operations misc_fops = {
>  	.owner		= THIS_MODULE,
> -	.open		= misc_open,
> +	.open		= misc_fops_open,
>  	.llseek		= noop_llseek,
>  };
>  
> @@ -266,9 +249,11 @@ static char *misc_devnode(struct device *dev, umode_t *mode)
>  static int __init misc_init(void)
>  {
>  	int err;
> +#ifdef CONFIG_PROC_FS
>  	struct proc_dir_entry *ret;
>  
> -	ret = proc_create_seq("misc", 0, NULL, &misc_seq_ops);
> +	ret = proc_create("misc", 0, NULL, &misc_proc_ops);
> +#endif

Any way to do this without the #ifdef in the middle of the function?
Otherwise it's not worth taking this change at all.

thanks,

greg k-h