Re: [PATCH] Fix proc_file_write missing ppos update

[Arnd Bergmann <arnd@arndb.de>]

On Monday 31 August 2009, Alexey Dobriyan wrote:
> Out-of-tree argument means almost nothing.
> 
> ...
>
> You'd better start converting to struct file_operations::write now.

Maybe a purely mechanical conversion to file_operations would be a nice first
step, just so we can remove read_proc and write_proc? Taking scsi_proc.c
as an example, this should be really straightforward and lets us get
rid of the write_proc and read_proc callbacks from proc_dir_entry without
having to rewrite all the remaining drivers that use it.

Obviously, someone who understands the specific driver code better should
then clean up the code by converting to seq_file operations or something
else that is appropriate there.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/scsi_proc.c |   27 ++++++++++++++++++++++-----
 fs/proc/generic.c        |   16 ++++++----------
 include/linux/proc_fs.h  |   11 +++++++++++
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/scsi_proc.c b/drivers/scsi/scsi_proc.c
index 77fbddb..f2a48a1 100644
--- a/drivers/scsi/scsi_proc.c
+++ b/drivers/scsi/scsi_proc.c
@@ -136,6 +136,26 @@ void scsi_proc_hostdir_rm(struct scsi_host_template *sht)
 	mutex_unlock(&global_host_template_mutex);
 }
 
+static ssize_t scsi_proc_read(struct file *file, char __user *buf,
+			size_t count, loff_t *off)
+{
+	return __proc_file_read(&proc_scsi_read, buf, count, off,
+			PDE(file->f_path.dentry->d_inode)->data);
+}
+
+static ssize_t scsi_proc_write(struct file *file, const char __user *buf,
+		size_t count, loff_t *off)
+{
+	return proc_scsi_write_proc(file, buf, count,
+			PDE(file->f_path.dentry->d_inode)->data);
+}
+
+static const struct file_operations scsi_proc_fops = {
+	.owner = THIS_MODULE,
+	.read = scsi_proc_read,
+	.write = scsi_proc_write,
+	.llseek = generic_file_llseek,
+};
 
 /**
  * scsi_proc_host_add - Add entry for this host to appropriate /proc dir
@@ -151,16 +170,14 @@ void scsi_proc_host_add(struct Scsi_Host *shost)
 		return;
 
 	sprintf(name,"%d", shost->host_no);
-	p = create_proc_read_entry(name, S_IFREG | S_IRUGO | S_IWUSR,
-			sht->proc_dir, proc_scsi_read, shost);
+	p = proc_create_data(name, S_IFREG | S_IRUGO | S_IWUSR,
+			sht->proc_dir, &scsi_proc_fops, shost);
 	if (!p) {
 		printk(KERN_ERR "%s: Failed to register host %d in"
 		       "%s\n", __func__, shost->host_no,
 		       sht->proc_name);
 		return;
-	} 
-
-	p->write_proc = proc_scsi_write_proc;
+	}
 }
 
 /**
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index fa678ab..ccf3949 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -36,17 +36,14 @@ static int proc_match(int len, const char *name, struct proc_dir_entry *de)
 /* buffer size is one page but our output routines use some slack for overruns */
 #define PROC_BLOCK_SIZE	(PAGE_SIZE - 1024)
 
-static ssize_t
-__proc_file_read(struct file *file, char __user *buf, size_t nbytes,
-	       loff_t *ppos)
+ssize_t __proc_file_read(read_proc_t read_proc, char __user *buf,
+			size_t nbytes, loff_t *ppos, void *data)
 {
-	struct inode * inode = file->f_path.dentry->d_inode;
 	char 	*page;
 	ssize_t	retval=0;
 	int	eof=0;
 	ssize_t	n, count;
 	char	*start;
-	struct proc_dir_entry * dp;
 	unsigned long long pos;
 
 	/*
@@ -60,7 +57,6 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 	if (nbytes > MAX_NON_LFS - pos)
 		nbytes = MAX_NON_LFS - pos;
 
-	dp = PDE(inode);
 	if (!(page = (char*) __get_free_page(GFP_TEMPORARY)))
 		return -ENOMEM;
 
@@ -68,7 +64,7 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 		count = min_t(size_t, PROC_BLOCK_SIZE, nbytes);
 
 		start = NULL;
-		if (dp->read_proc) {
+		if (read_proc) {
 			/*
 			 * How to be a proc read function
 			 * ------------------------------
@@ -116,8 +112,8 @@ __proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 			 *    requested offset advanced by the number of bytes
 			 *    absorbed.
 			 */
-			n = dp->read_proc(page, &start, *ppos,
-					  count, &eof, dp->data);
+			n = read_proc(page, &start, *ppos,
+					  count, &eof, data);
 		} else
 			break;
 
@@ -197,7 +193,7 @@ proc_file_read(struct file *file, char __user *buf, size_t nbytes,
 	pde->pde_users++;
 	spin_unlock(&pde->pde_unload_lock);
 
-	rv = __proc_file_read(file, buf, nbytes, ppos);
+	rv = __proc_file_read(pde->read_proc, buf, nbytes, ppos, pde->data);
 
 	pde_users_dec(pde);
 	return rv;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index e6e77d3..949a382 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -103,6 +103,10 @@ struct proc_dir_entry *proc_create_data(const char *name, mode_t mode,
 				struct proc_dir_entry *parent,
 				const struct file_operations *proc_fops,
 				void *data);
+/* do not use in new code */
+ssize_t __proc_file_read(read_proc_t read_proc, char __user *buf,
+                        size_t nbytes, loff_t *ppos, void *data);
+
 extern void remove_proc_entry(const char *name, struct proc_dir_entry *parent);
 
 struct pid_namespace;
@@ -195,6 +199,13 @@ static inline struct proc_dir_entry *proc_create_data(const char *name,
 }
 #define remove_proc_entry(name, parent) do {} while (0)
 
+static inline ssize_t __proc_file_read(read_proc_t read_proc,
+			char __user *buf, size_t nbytes, loff_t *ppos,
+			void *data)
+{
+	return 0;
+}
+
 static inline struct proc_dir_entry *proc_symlink(const char *name,
 		struct proc_dir_entry *parent,const char *dest) {return NULL;}
 static inline struct proc_dir_entry *proc_mkdir(const char *name,