To simplify the filedesc fd_ofileflags handling, how about if we make 
falloc() take optional flags to OR into the new fd's ofileflags? That 
eliminates a quarter of the references to fd_ofileflags.

While here, make falloc() assert that its return arguments are non-NULL, 
as it doesn't make sense to allocate a larval file but no get a reference 
back with which to mature it, or to allocate an fd but not get back which.

ok?

Philip Guenther


Index: share/man/man9/file.9
===================================================================
RCS file: /data/src/openbsd/src/share/man/man9/file.9,v
retrieving revision 1.16
diff -u -p -r1.16 file.9
--- share/man/man9/file.9       23 Nov 2015 17:53:57 -0000      1.16
+++ share/man/man9/file.9       11 Feb 2017 08:32:51 -0000
@@ -38,7 +38,7 @@
 .In sys/file.h
 .In sys/filedesc.h
 .Ft int
-.Fn falloc "struct proc *p" "struct file **resultfp" "int *resultfd"
+.Fn falloc "struct proc *p" "int flags" "struct file **resultfp" "int 
*resultfd"
 .Ft int
 .Fn fdrelease "struct proc *p" "int fd"
 .Ft void
@@ -66,17 +66,32 @@ kqueues (see
 .Xr kqueue 2 ) ,
 and various special purpose communication endpoints.
 .Pp
-A new file descriptor is allocated with the function
+A new file and a file descriptor for it are allocated with the function
+.Fn falloc .
+The
+.Fa flags
+argument can be used to set the
+.Dv UF_EXCLOSE
+flag on the new descriptor.
+The larval file and fd are returned via
+.Fa resultfp
+and
+.Fa resultfd ,
+which must not be
+.Dv NULL .
 .Fn falloc
-and freed with
+initializes the new file to have a reference count of two:
+one for the reference from the file descriptor table and one
+for the caller to release with
+.Fn FRELE
+when it done initializing it.
+.Pp
+A file descriptor is freed with
 .Fn fdrelease .
-.Fn falloc
-and
-.Fn fdrelease
-deal with allocating and freeing slots in the file descriptor table,
-expanding the table when necessary and initializing the descriptor.
-It's possible to do those things in smaller steps, but it's not
-recommended to make complicated kernel APIs that require it.
+This releases the reference that it holds to the underlying file;
+if that's the last reference then the file will be freed.
+.\" with
+.\" .Xr closef 9 .
 .Pp
 The files are extracted from the file descriptor table using the
 functions
Index: sys/sys/filedesc.h
===================================================================
RCS file: /data/src/openbsd/src/sys/sys/filedesc.h,v
retrieving revision 1.33
diff -u -p -r1.33 filedesc.h
--- sys/sys/filedesc.h  25 Jan 2017 06:15:50 -0000      1.33
+++ sys/sys/filedesc.h  11 Feb 2017 09:13:34 -0000
@@ -125,7 +125,7 @@ void        filedesc_init(void);
 int    dupfdopen(struct proc *, int, int);
 int    fdalloc(struct proc *p, int want, int *result);
 void   fdexpand(struct proc *);
-int    falloc(struct proc *p, struct file **resultfp, int *resultfd);
+int    falloc(struct proc *_p, int _flags, struct file **_rfp, int *_rfd);
 struct filedesc *fdinit(void);
 struct filedesc *fdshare(struct process *);
 struct filedesc *fdcopy(struct process *);
Index: sys/kern/exec_script.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/exec_script.c,v
retrieving revision 1.39
diff -u -p -r1.39 exec_script.c
--- sys/kern/exec_script.c      25 Apr 2016 20:00:33 -0000      1.39
+++ sys/kern/exec_script.c      11 Feb 2017 09:12:08 -0000
@@ -169,7 +169,7 @@ check_shell:
 #endif
 
                fdplock(p->p_fd);
-               error = falloc(p, &fp, &epp->ep_fd);
+               error = falloc(p, 0, &fp, &epp->ep_fd);
                fdpunlock(p->p_fd);
                if (error)
                        goto fail;
Index: sys/kern/kern_descrip.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/kern_descrip.c,v
retrieving revision 1.139
diff -u -p -r1.139 kern_descrip.c
--- sys/kern/kern_descrip.c     24 Jan 2017 04:09:59 -0000      1.139
+++ sys/kern/kern_descrip.c     11 Feb 2017 09:08:23 -0000
@@ -895,11 +895,14 @@ fdexpand(struct proc *p)
  * a file descriptor for the process that refers to it.
  */
 int
-falloc(struct proc *p, struct file **resultfp, int *resultfd)
+falloc(struct proc *p, int flags, struct file **resultfp, int *resultfd)
 {
        struct file *fp, *fq;
        int error, i;
 
+       KASSERT(resultfp != NULL);
+       KASSERT(resultfd != NULL);
+
        fdpassertlocked(p->p_fd);
 restart:
        if ((error = fdalloc(p, 0, &i)) != 0) {
@@ -929,13 +932,12 @@ restart:
                LIST_INSERT_HEAD(&filehead, fp, f_list);
        }
        p->p_fd->fd_ofiles[i] = fp;
+       p->p_fd->fd_ofileflags[i] |= (flags & UF_EXCLOSE);
        fp->f_count = 1;
        fp->f_cred = p->p_ucred;
        crhold(fp->f_cred);
-       if (resultfp)
-               *resultfp = fp;
-       if (resultfd)
-               *resultfd = i;
+       *resultfp = fp;
+       *resultfd = i;
        FREF(fp);
        return (0);
 }
Index: sys/kern/kern_event.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/kern_event.c,v
retrieving revision 1.77
diff -u -p -r1.77 kern_event.c
--- sys/kern/kern_event.c       24 Sep 2016 18:39:17 -0000      1.77
+++ sys/kern/kern_event.c       11 Feb 2017 09:08:29 -0000
@@ -440,7 +440,7 @@ sys_kqueue(struct proc *p, void *v, regi
        int fd, error;
 
        fdplock(fdp);
-       error = falloc(p, &fp, &fd);
+       error = falloc(p, 0, &fp, &fd);
        fdpunlock(fdp);
        if (error)
                return (error);
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/kern_exec.c,v
retrieving revision 1.186
diff -u -p -r1.186 kern_exec.c
--- sys/kern/kern_exec.c        8 Feb 2017 20:58:30 -0000       1.186
+++ sys/kern/kern_exec.c        11 Feb 2017 09:08:36 -0000
@@ -576,7 +576,7 @@ sys_execve(struct proc *p, void *v, regi
                                struct vnode *vp;
                                int indx;
 
-                               if ((error = falloc(p, &fp, &indx)) != 0)
+                               if ((error = falloc(p, 0, &fp, &indx)) != 0)
                                        break;
 #ifdef DIAGNOSTIC
                                if (indx != i)
Index: sys/kern/sys_pipe.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/sys_pipe.c,v
retrieving revision 1.75
diff -u -p -r1.75 sys_pipe.c
--- sys/kern/sys_pipe.c 8 Oct 2016 02:16:43 -0000       1.75
+++ sys/kern/sys_pipe.c 11 Feb 2017 09:09:22 -0000
@@ -133,7 +133,9 @@ dopipe(struct proc *p, int *ufds, int fl
        struct filedesc *fdp = p->p_fd;
        struct file *rf, *wf;
        struct pipe *rpipe, *wpipe = NULL;
-       int fds[2], error;
+       int fds[2], cloexec, error;
+
+       cloexec = (flags & O_CLOEXEC) ? UF_EXCLOSE : 0;
 
        rpipe = pool_get(&pipe_pool, PR_WAITOK);
        error = pipe_create(rpipe);
@@ -146,7 +148,7 @@ dopipe(struct proc *p, int *ufds, int fl
 
        fdplock(fdp);
 
-       error = falloc(p, &rf, &fds[0]);
+       error = falloc(p, cloexec, &rf, &fds[0]);
        if (error != 0)
                goto free2;
        rf->f_flag = FREAD | FWRITE | (flags & FNONBLOCK);
@@ -154,18 +156,13 @@ dopipe(struct proc *p, int *ufds, int fl
        rf->f_data = rpipe;
        rf->f_ops = &pipeops;
 
-       error = falloc(p, &wf, &fds[1]);
+       error = falloc(p, cloexec, &wf, &fds[1]);
        if (error != 0)
                goto free3;
        wf->f_flag = FREAD | FWRITE | (flags & FNONBLOCK);
        wf->f_type = DTYPE_PIPE;
        wf->f_data = wpipe;
        wf->f_ops = &pipeops;
-
-       if (flags & O_CLOEXEC) {
-               fdp->fd_ofileflags[fds[0]] |= UF_EXCLOSE;
-               fdp->fd_ofileflags[fds[1]] |= UF_EXCLOSE;
-       }
 
        rpipe->pipe_peer = wpipe;
        wpipe->pipe_peer = rpipe;
Index: sys/kern/tty_pty.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/tty_pty.c,v
retrieving revision 1.78
diff -u -p -r1.78 tty_pty.c
--- sys/kern/tty_pty.c  24 May 2016 16:09:07 -0000      1.78
+++ sys/kern/tty_pty.c  11 Feb 2017 09:09:35 -0000
@@ -1055,11 +1055,11 @@ ptmioctl(dev_t dev, u_long cmd, caddr_t 
        case PTMGET:
                fdplock(fdp);
                /* Grab two filedescriptors. */
-               if ((error = falloc(p, &cfp, &cindx)) != 0) {
+               if ((error = falloc(p, 0, &cfp, &cindx)) != 0) {
                        fdpunlock(fdp);
                        break;
                }
-               if ((error = falloc(p, &sfp, &sindx)) != 0) {
+               if ((error = falloc(p, 0, &sfp, &sindx)) != 0) {
                        fdremove(fdp, cindx);
                        closef(cfp, p);
                        fdpunlock(fdp);
Index: sys/kern/uipc_syscalls.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.148
diff -u -p -r1.148 uipc_syscalls.c
--- sys/kern/uipc_syscalls.c    26 Jan 2017 01:58:00 -0000      1.148
+++ sys/kern/uipc_syscalls.c    11 Feb 2017 09:16:00 -0000
@@ -94,9 +94,7 @@ sys_socket(struct proc *p, void *v, regi
                return (error);
 
        fdplock(fdp);
-       error = falloc(p, &fp, &fd);
-       if (error == 0 && (type & SOCK_CLOEXEC))
-               fdp->fd_ofileflags[fd] |= UF_EXCLOSE;
+       error = falloc(p, (type & SOCK_CLOEXEC) ? UF_EXCLOSE : 0, &fp, &fd);
        fdpunlock(fdp);
        if (error != 0)
                goto out;
@@ -279,9 +277,7 @@ doaccept(struct proc *p, int sock, struc
        headfp = fp;
 
        fdplock(fdp);
-       error = falloc(p, &fp, &tmpfd);
-       if (!error && (flags & SOCK_CLOEXEC))
-               fdp->fd_ofileflags[tmpfd] |= UF_EXCLOSE;
+       error = falloc(p, (flags & SOCK_CLOEXEC) ? UF_EXCLOSE : 0, &fp, &tmpfd);
        fdpunlock(fdp);
        if (error) {
                FRELE(headfp, p);
@@ -447,11 +443,12 @@ sys_socketpair(struct proc *p, void *v, 
        struct filedesc *fdp = p->p_fd;
        struct file *fp1, *fp2;
        struct socket *so1, *so2;
-       int type, flags, fflag, error, sv[2];
+       int type, cloexec, nonblock, fflag, error, sv[2];
 
        type  = SCARG(uap, type) & ~(SOCK_CLOEXEC | SOCK_NONBLOCK);
-       flags = SCARG(uap, type) &  (SOCK_CLOEXEC | SOCK_NONBLOCK);
-       fflag = FREAD | FWRITE | (flags & SOCK_NONBLOCK ? FNONBLOCK : 0);
+       cloexec = (SCARG(uap, type) & SOCK_CLOEXEC) ? UF_EXCLOSE : 0;
+       nonblock = SCARG(uap, type) &  SOCK_NONBLOCK;
+       fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
 
        error = socreate(SCARG(uap, domain), &so1, type, SCARG(uap, protocol));
        if (error)
@@ -471,32 +468,28 @@ sys_socketpair(struct proc *p, void *v, 
                        goto free2;
        }
        fdplock(fdp);
-       if ((error = falloc(p, &fp1, &sv[0])) != 0)
+       if ((error = falloc(p, cloexec, &fp1, &sv[0])) != 0)
                goto free3;
        fp1->f_flag = fflag;
        fp1->f_type = DTYPE_SOCKET;
        fp1->f_ops = &socketops;
        fp1->f_data = so1;
-       if ((error = falloc(p, &fp2, &sv[1])) != 0)
+       if ((error = falloc(p, cloexec, &fp2, &sv[1])) != 0)
                goto free4;
        fp2->f_flag = fflag;
        fp2->f_type = DTYPE_SOCKET;
        fp2->f_ops = &socketops;
        fp2->f_data = so2;
-       if (flags & SOCK_CLOEXEC) {
-               fdp->fd_ofileflags[sv[0]] |= UF_EXCLOSE;
-               fdp->fd_ofileflags[sv[1]] |= UF_EXCLOSE;
-       }
        error = copyout(sv, SCARG(uap, rsv), 2 * sizeof (int));
        if (error == 0) {
 #ifdef KTRACE
                if (KTRPOINT(p, KTR_STRUCT))
                        ktrfds(p, sv, 2);
 #endif
-               if (flags & SOCK_NONBLOCK) {
-                       (*fp1->f_ops->fo_ioctl)(fp1, FIONBIO, (caddr_t)&flags,
+               if (nonblock) {
+                       (*fp1->f_ops->fo_ioctl)(fp1, FIONBIO, (caddr_t)&type,
                            p);
-                       (*fp2->f_ops->fo_ioctl)(fp2, FIONBIO, (caddr_t)&flags,
+                       (*fp2->f_ops->fo_ioctl)(fp2, FIONBIO, (caddr_t)&type,
                            p);
                }
                FILE_SET_MATURE(fp1, p);
Index: sys/kern/vfs_syscalls.c
===================================================================
RCS file: /data/src/openbsd/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.269
diff -u -p -r1.269 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c     23 Jan 2017 22:34:10 -0000      1.269
+++ sys/kern/vfs_syscalls.c     11 Feb 2017 09:13:16 -0000
@@ -802,7 +802,8 @@ doopenat(struct proc *p, int fd, const c
 
        fdplock(fdp);
 
-       if ((error = falloc(p, &fp, &indx)) != 0)
+       if ((error = falloc(p, (oflags & O_CLOEXEC) ? UF_EXCLOSE : 0, &fp,
+           &indx)) != 0)
                goto out;
        flags = FFLAGS(oflags);
        if (flags & FREAD)
@@ -812,9 +813,6 @@ doopenat(struct proc *p, int fd, const c
        if (oflags & O_CREAT)
                ni_pledge |= PLEDGE_CPATH;
 
-       if (flags & O_CLOEXEC)
-               fdp->fd_ofileflags[indx] |= UF_EXCLOSE;
-
        cmode = ((mode &~ fdp->fd_cmask) & ALLPERMS) &~ S_ISTXT;
        if ((p->p_p->ps_flags & PS_PLEDGE))
                cmode &= ACCESSPERMS;
@@ -970,12 +968,11 @@ sys_fhopen(struct proc *p, void *v, regi
                return (EINVAL);
 
        fdplock(fdp);
-       if ((error = falloc(p, &fp, &indx)) != 0) {
+       if ((error = falloc(p, (flags & O_CLOEXEC) ? UF_EXCLOSE : 0, &fp,
+           &indx)) != 0) {
                fp = NULL;
                goto bad;
        }
-       if (flags & O_CLOEXEC)
-               fdp->fd_ofileflags[indx] |= UF_EXCLOSE;
 
        if ((error = copyin(SCARG(uap, fhp), &fh, sizeof(fhandle_t))) != 0)
                goto bad;

Reply via email to