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@xxxxxxx>
> 
> 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@xxxxxxx>
> ---
> 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