Several cleanups related to pipe(2).

- Use `fildes[2]' instead of `*fildes' to make more clear that pipe(2)
  fills an array with two descriptors.

- Remove EFAULT from the manual page. Because of the current calling
  convention, pipe(2) raises a segmentation fault when an invalid
  address is passed.

- Introduce kern_pipe() to make it easier for binary emulations to
  implement pipe(2).

- Make Linux binary emulation use kern_pipe(), which means we don't have
  to recover td_retval after calling the FreeBSD system call.

Approved by:	rdivacky
Discussed on:	arch

5 files changed, 34 insertions, 48 deletions

diff --git a/lib/libc/sys/pipe.2 b/lib/libc/sys/pipe.2
index faf1a91..92d137f 100644
--- a/lib/libc/sys/pipe.2
+++ b/lib/libc/sys/pipe.2
@@ -39,7 +39,7 @@
 .Sh SYNOPSIS
 .In unistd.h
 .Ft int
-.Fn pipe "int *fildes"
+.Fn pipe "int fildes[2]"
 .Sh DESCRIPTION
 The
 .Fn pipe
@@ -96,11 +96,6 @@ Too many descriptors are active.
 The system file table is full.
 .It Bq Er ENOMEM
 Not enough kernel memory to establish a pipe.
-.It Bq Er EFAULT
-The
-.Fa fildes
-buffer is in an invalid area of the process's address
-space.
 .El
 .Sh SEE ALSO
 .Xr sh 1 ,
diff --git a/sys/amd64/linux32/linux32_machdep.c b/sys/amd64/linux32/linux32_machdep.c
index aecf869..fcf01c8 100644
--- a/sys/amd64/linux32/linux32_machdep.c
+++ b/sys/amd64/linux32/linux32_machdep.c
@@ -977,33 +977,20 @@ linux_iopl(struct thread *td, struct linux_iopl_args *args)
 int
 linux_pipe(struct thread *td, struct linux_pipe_args *args)
 {
-	int pip[2];
 	int error;
-	register_t reg_rdx;
+	int fildes[2];
 
 #ifdef DEBUG
 	if (ldebug(pipe))
 		printf(ARGS(pipe, "*"));
 #endif
 
-	reg_rdx = td->td_retval[1];
-	error = pipe(td, 0);
-	if (error) {
-		td->td_retval[1] = reg_rdx;
-		return (error);
-	}
-
-	pip[0] = td->td_retval[0];
-	pip[1] = td->td_retval[1];
-	error = copyout(pip, args->pipefds, 2 * sizeof(int));
-	if (error) {
-		td->td_retval[1] = reg_rdx;
+	error = kern_pipe(td, fildes);
+	if (error)
 		return (error);
-	}
 
-	td->td_retval[1] = reg_rdx;
-	td->td_retval[0] = 0;
-	return (0);
+	/* XXX: Close descriptors on error. */
+	return (copyout(fildes, args->pipefds, sizeof fildes));
 }
 
 int
diff --git a/sys/i386/linux/linux_machdep.c b/sys/i386/linux/linux_machdep.c
index ded3b17..7f727ac 100644
--- a/sys/i386/linux/linux_machdep.c
+++ b/sys/i386/linux/linux_machdep.c
@@ -813,29 +813,19 @@ int
 linux_pipe(struct thread *td, struct linux_pipe_args *args)
 {
 	int error;
-	int reg_edx;
+	int fildes[2];
 
 #ifdef DEBUG
 	if (ldebug(pipe))
 		printf(ARGS(pipe, "*"));
 #endif
 
-	reg_edx = td->td_retval[1];
-	error = pipe(td, 0);
-	if (error) {
-		td->td_retval[1] = reg_edx;
-		return (error);
-	}
-
-	error = copyout(td->td_retval, args->pipefds, 2*sizeof(int));
-	if (error) {
-		td->td_retval[1] = reg_edx;
+	error = kern_pipe(td, fildes);
+	if (error)
 		return (error);
-	}
 
-	td->td_retval[1] = reg_edx;
-	td->td_retval[0] = 0;
-	return (0);
+	/* XXX: Close descriptors on error. */
+	return (copyout(fildes, args->pipefds, sizeof fildes));
 }
 
 int
diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index b8c1dfe..9b343fe 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -108,6 +108,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/poll.h>
 #include <sys/selinfo.h>
 #include <sys/signalvar.h>
+#include <sys/syscallsubr.h>
 #include <sys/sysctl.h>
 #include <sys/sysproto.h>
 #include <sys/pipe.h>
@@ -307,13 +308,8 @@ pipe_zone_fini(void *mem, int size)
  * The pipe system call for the DTYPE_PIPE type of pipes.  If we fail, let
  * the zone pick up the pieces via pipeclose().
  */
-/* ARGSUSED */
 int
-pipe(td, uap)
-	struct thread *td;
-	struct pipe_args /* {
-		int	dummy;
-	} */ *uap;
+kern_pipe(struct thread *td, int fildes[2])
 {
 	struct filedesc *fdp = td->td_proc->p_fd;
 	struct file *rf, *wf;
@@ -357,7 +353,7 @@ pipe(td, uap)
 		return (error);
 	}
 	/* An extra reference on `rf' has been held for us by falloc(). */
-	td->td_retval[0] = fd;
+	fildes[0] = fd;
 
 	/*
 	 * Warning: once we've gotten past allocation of the fd for the
@@ -368,7 +364,7 @@ pipe(td, uap)
 	finit(rf, FREAD | FWRITE, DTYPE_PIPE, rpipe, &pipeops);
 	error = falloc(td, &wf, &fd);
 	if (error) {
-		fdclose(fdp, rf, td->td_retval[0], td);
+		fdclose(fdp, rf, fildes[0], td);
 		fdrop(rf, td);
 		/* rpipe has been closed by fdrop(). */
 		pipeclose(wpipe);
@@ -377,12 +373,29 @@ pipe(td, uap)
 	/* An extra reference on `wf' has been held for us by falloc(). */
 	finit(wf, FREAD | FWRITE, DTYPE_PIPE, wpipe, &pipeops);
 	fdrop(wf, td);
-	td->td_retval[1] = fd;
+	fildes[1] = fd;
 	fdrop(rf, td);
 
 	return (0);
 }
 
+/* ARGSUSED */
+int
+pipe(struct thread *td, struct pipe_args *uap)
+{
+	int error;
+	int fildes[2];
+
+	error = kern_pipe(td, fildes);
+	if (error)
+		return (error);
+	
+	td->td_retval[0] = fildes[0];
+	td->td_retval[1] = fildes[1];
+
+	return (0);
+}
+
 /*
  * Allocate kva for pipe circular buffer, the space is pageable
  * This routine will 'realloc' the size of a pipe safely, if it fails
diff --git a/sys/sys/syscallsubr.h b/sys/sys/syscallsubr.h
index 0708ea1..bcd244f 100644
--- a/sys/sys/syscallsubr.h
+++ b/sys/sys/syscallsubr.h
@@ -142,6 +142,7 @@ int	kern_openat(struct thread *td, int fd, char *path,
 	    enum uio_seg pathseg, int flags, int mode);
 int	kern_pathconf(struct thread *td, char *path, enum uio_seg pathseg,
 	    int name);
+int	kern_pipe(struct thread *td, int fildes[2]);
 int	kern_preadv(struct thread *td, int fd, struct uio *auio, off_t offset);
 int	kern_ptrace(struct thread *td, int req, pid_t pid, void *addr,
 	    int data);