[PATCH] Some cleanup in the pipe code

Split the big and hard to read do_pipe function into smaller pieces. This creates new create_write_pipe/free_write_pipe/create_read_pipe functions. These functions are made global so that they can be used by other parts of the kernel. The resulting code is more generic and easier to read and has cleaner error handling and less gotos. [akpm@osdl.org: cleanup] Signed-off-by: Andi Kleen <ak@suse.de> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] Some cleanup in the pipe code
Split the big and hard to read do_pipe function into smaller pieces. This creates new create_write_pipe/free_write_pipe/create_read_pipe functions. These functions are made global so that they can be used by other parts of the kernel. The resulting code is more generic and easier to read and has cleaner error handling and less gotos. [akpm@osdl.org: cleanup] Signed-off-by: Andi Kleen <ak@suse.de> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Andi Kleen · Linus Torvalds
1 parent 65da4d81f4
Showing 2 changed files with 96 additions and 62 deletions Side-by-side Diff
fs/pipe.c
include/linux/fs.h
@@ -874,87 +874,118 @@
 	return NULL;
 }
  
-int do_pipe(int *fd)
+struct file *create_write_pipe(void)
 {
-	struct qstr this;
-	char name[32];
+	int err;
+	struct inode *inode;
+	struct file *f;
 	struct dentry *dentry;
-	struct inode * inode;
-	struct file *f1, *f2;
-	int error;
-	int i, j;
+	char name[32];
+	struct qstr this;
  
-	error = -ENFILE;
-	f1 = get_empty_filp();
-	if (!f1)
-		goto no_files;
-
-	f2 = get_empty_filp();
-	if (!f2)
-		goto close_f1;
-
+	f = get_empty_filp();
+	if (!f)
+		return ERR_PTR(-ENFILE);
+	err = -ENFILE;
 	inode = get_pipe_inode();
 	if (!inode)
-		goto close_f12;
+		goto err_file;
  
-	error = get_unused_fd();
-	if (error < 0)
-		goto close_f12_inode;
-	i = error;
-
-	error = get_unused_fd();
-	if (error < 0)
-		goto close_f12_inode_i;
-	j = error;
-
-	error = -ENOMEM;
 	sprintf(name, "[%lu]", inode->i_ino);
 	this.name = name;
 	this.len = strlen(name);
 	this.hash = inode->i_ino; /* will go */
+	err = -ENOMEM;
 	dentry = d_alloc(pipe_mnt->mnt_sb->s_root, &this);
 	if (!dentry)
-		goto close_f12_inode_i_j;
+		goto err_inode;
  
 	dentry->d_op = &pipefs_dentry_operations;
 	d_add(dentry, inode);
-	f1->f_vfsmnt = f2->f_vfsmnt = mntget(mntget(pipe_mnt));
-	f1->f_dentry = f2->f_dentry = dget(dentry);
-	f1->f_mapping = f2->f_mapping = inode->i_mapping;
+	f->f_vfsmnt = mntget(pipe_mnt);
+	f->f_dentry = dentry;
+	f->f_mapping = inode->i_mapping;
  
-	/* read file */
-	f1->f_pos = f2->f_pos = 0;
-	f1->f_flags = O_RDONLY;
-	f1->f_op = &read_pipe_fops;
-	f1->f_mode = FMODE_READ;
-	f1->f_version = 0;
+	f->f_flags = O_WRONLY;
+	f->f_op = &write_pipe_fops;
+	f->f_mode = FMODE_WRITE;
+	f->f_version = 0;
  
-	/* write file */
-	f2->f_flags = O_WRONLY;
-	f2->f_op = &write_pipe_fops;
-	f2->f_mode = FMODE_WRITE;
-	f2->f_version = 0;
+	return f;
  
-	fd_install(i, f1);
-	fd_install(j, f2);
-	fd[0] = i;
-	fd[1] = j;
+ err_inode:
+	free_pipe_info(inode);
+	iput(inode);
+ err_file:
+	put_filp(f);
+	return ERR_PTR(err);
+}
  
+void free_write_pipe(struct file *f)
+{
+	mntput(f->f_vfsmnt);
+	dput(f->f_dentry);
+	put_filp(f);
+}
+
+struct file *create_read_pipe(struct file *wrf)
+{
+	struct file *f = get_empty_filp();
+	if (!f)
+		return ERR_PTR(-ENFILE);
+
+	/* Grab pipe from the writer */
+	f->f_vfsmnt = mntget(wrf->f_vfsmnt);
+	f->f_dentry = dget(wrf->f_dentry);
+	f->f_mapping = wrf->f_dentry->d_inode->i_mapping;
+
+	f->f_pos = 0;
+	f->f_flags = O_RDONLY;
+	f->f_op = &read_pipe_fops;
+	f->f_mode = FMODE_READ;
+	f->f_version = 0;
+
+	return f;
+}
+
+int do_pipe(int *fd)
+{
+	struct file *fw, *fr;
+	int error;
+	int fdw, fdr;
+
+	fw = create_write_pipe();
+	if (IS_ERR(fw))
+		return PTR_ERR(fw);
+	fr = create_read_pipe(fw);
+	error = PTR_ERR(fr);
+	if (IS_ERR(fr))
+		goto err_write_pipe;
+
+	error = get_unused_fd();
+	if (error < 0)
+		goto err_read_pipe;
+	fdr = error;
+
+	error = get_unused_fd();
+	if (error < 0)
+		goto err_fdr;
+	fdw = error;
+
+	fd_install(fdr, fr);
+	fd_install(fdw, fw);
+	fd[0] = fdr;
+	fd[1] = fdw;
+
 	return 0;
  
-close_f12_inode_i_j:
-	put_unused_fd(j);
-close_f12_inode_i:
-	put_unused_fd(i);
-close_f12_inode:
-	free_pipe_info(inode);
-	iput(inode);
-close_f12:
-	put_filp(f2);
-close_f1:
-	put_filp(f1);
-no_files:
-	return error;	
+ err_fdr:
+	put_unused_fd(fdr);
+ err_read_pipe:
+	put_filp(fr);
+ err_write_pipe:
+	free_write_pipe(fw);
+	return error;
 }
  
 /*
@@ -1641,6 +1641,9 @@
 		atomic_inc(&file->f_dentry->d_inode->i_writecount);
 }
 extern int do_pipe(int *);
+extern struct file *create_read_pipe(struct file *f);
+extern struct file *create_write_pipe(void);
+extern void free_write_pipe(struct file *);
  
 extern int open_namei(int dfd, const char *, int, int, struct nameidata *);
 extern int may_open(struct nameidata *, int, int);
...	...	@@ -874,87 +874,118 @@
874	874	return NULL;
875	875	}
876	876
877		-int do_pipe(int *fd)
	877	+struct file *create_write_pipe(void)
878	878	{
879		- struct qstr this;
880		- char name[32];
	879	+ int err;
	880	+ struct inode *inode;
	881	+ struct file *f;
881	882	struct dentry *dentry;
882		- struct inode * inode;
883		- struct file f1, f2;
884		- int error;
885		- int i, j;
	883	+ char name[32];
	884	+ struct qstr this;
886	885
887		- error = -ENFILE;
888		- f1 = get_empty_filp();
889		- if (!f1)
890		- goto no_files;
891		-
892		- f2 = get_empty_filp();
893		- if (!f2)
894		- goto close_f1;
895		-
	886	+ f = get_empty_filp();
	887	+ if (!f)
	888	+ return ERR_PTR(-ENFILE);
	889	+ err = -ENFILE;
896	890	inode = get_pipe_inode();
897	891	if (!inode)
898		- goto close_f12;
	892	+ goto err_file;
899	893
900		- error = get_unused_fd();
901		- if (error < 0)
902		- goto close_f12_inode;
903		- i = error;
904		-
905		- error = get_unused_fd();
906		- if (error < 0)
907		- goto close_f12_inode_i;
908		- j = error;
909		-
910		- error = -ENOMEM;
911	894	sprintf(name, "[%lu]", inode->i_ino);
912	895	this.name = name;
913	896	this.len = strlen(name);
914	897	this.hash = inode->i_ino; /* will go */
	898	+ err = -ENOMEM;
915	899	dentry = d_alloc(pipe_mnt->mnt_sb->s_root, &this);
916	900	if (!dentry)
917		- goto close_f12_inode_i_j;
	901	+ goto err_inode;
918	902
919	903	dentry->d_op = &pipefs_dentry_operations;
920	904	d_add(dentry, inode);
921		- f1->f_vfsmnt = f2->f_vfsmnt = mntget(mntget(pipe_mnt));
922		- f1->f_dentry = f2->f_dentry = dget(dentry);
923		- f1->f_mapping = f2->f_mapping = inode->i_mapping;
	905	+ f->f_vfsmnt = mntget(pipe_mnt);
	906	+ f->f_dentry = dentry;
	907	+ f->f_mapping = inode->i_mapping;
924	908
925		- /* read file */
926		- f1->f_pos = f2->f_pos = 0;
927		- f1->f_flags = O_RDONLY;
928		- f1->f_op = &read_pipe_fops;
929		- f1->f_mode = FMODE_READ;
930		- f1->f_version = 0;
	909	+ f->f_flags = O_WRONLY;
	910	+ f->f_op = &write_pipe_fops;
	911	+ f->f_mode = FMODE_WRITE;
	912	+ f->f_version = 0;
931	913
932		- /* write file */
933		- f2->f_flags = O_WRONLY;
934		- f2->f_op = &write_pipe_fops;
935		- f2->f_mode = FMODE_WRITE;
936		- f2->f_version = 0;
	914	+ return f;
937	915
938		- fd_install(i, f1);
939		- fd_install(j, f2);
940		- fd[0] = i;
941		- fd[1] = j;
	916	+ err_inode:
	917	+ free_pipe_info(inode);
	918	+ iput(inode);
	919	+ err_file:
	920	+ put_filp(f);
	921	+ return ERR_PTR(err);
	922	+}
942	923
	924	+void free_write_pipe(struct file *f)
	925	+{
	926	+ mntput(f->f_vfsmnt);
	927	+ dput(f->f_dentry);
	928	+ put_filp(f);
	929	+}
	930	+
	931	+struct file create_read_pipe(struct file wrf)
	932	+{
	933	+ struct file *f = get_empty_filp();
	934	+ if (!f)
	935	+ return ERR_PTR(-ENFILE);
	936	+
	937	+ /* Grab pipe from the writer */
	938	+ f->f_vfsmnt = mntget(wrf->f_vfsmnt);
	939	+ f->f_dentry = dget(wrf->f_dentry);
	940	+ f->f_mapping = wrf->f_dentry->d_inode->i_mapping;
	941	+
	942	+ f->f_pos = 0;
	943	+ f->f_flags = O_RDONLY;
	944	+ f->f_op = &read_pipe_fops;
	945	+ f->f_mode = FMODE_READ;
	946	+ f->f_version = 0;
	947	+
	948	+ return f;
	949	+}
	950	+
	951	+int do_pipe(int *fd)
	952	+{
	953	+ struct file fw, fr;
	954	+ int error;
	955	+ int fdw, fdr;
	956	+
	957	+ fw = create_write_pipe();
	958	+ if (IS_ERR(fw))
	959	+ return PTR_ERR(fw);
	960	+ fr = create_read_pipe(fw);
	961	+ error = PTR_ERR(fr);
	962	+ if (IS_ERR(fr))
	963	+ goto err_write_pipe;
	964	+
	965	+ error = get_unused_fd();
	966	+ if (error < 0)
	967	+ goto err_read_pipe;
	968	+ fdr = error;
	969	+
	970	+ error = get_unused_fd();
	971	+ if (error < 0)
	972	+ goto err_fdr;
	973	+ fdw = error;
	974	+
	975	+ fd_install(fdr, fr);
	976	+ fd_install(fdw, fw);
	977	+ fd[0] = fdr;
	978	+ fd[1] = fdw;
	979	+
943	980	return 0;
944	981
945		-close_f12_inode_i_j:
946		- put_unused_fd(j);
947		-close_f12_inode_i:
948		- put_unused_fd(i);
949		-close_f12_inode:
950		- free_pipe_info(inode);
951		- iput(inode);
952		-close_f12:
953		- put_filp(f2);
954		-close_f1:
955		- put_filp(f1);
956		-no_files:
957		- return error;
	982	+ err_fdr:
	983	+ put_unused_fd(fdr);
	984	+ err_read_pipe:
	985	+ put_filp(fr);
	986	+ err_write_pipe:
	987	+ free_write_pipe(fw);
	988	+ return error;
958	989	}
959	990
960	991	/*
...	...	@@ -1641,6 +1641,9 @@
1641	1641	atomic_inc(&file->f_dentry->d_inode->i_writecount);
1642	1642	}
1643	1643	extern int do_pipe(int *);
	1644	+extern struct file create_read_pipe(struct file f);
	1645	+extern struct file *create_write_pipe(void);
	1646	+extern void free_write_pipe(struct file *);
1644	1647
1645	1648	extern int open_namei(int dfd, const char , int, int, struct nameidata );
1646	1649	extern int may_open(struct nameidata *, int, int);