svn commit: r285357 - in head/sys: kern sys
Mateusz Guzik
mjg at FreeBSD.org
Fri Jul 10 13:54:04 UTC 2015
Author: mjg
Date: Fri Jul 10 13:54:03 2015
New Revision: 285357
URL: https://svnweb.freebsd.org/changeset/base/285357
Log:
fd: further cleanup of kern_dup
- make mode enum start from 0 so that the assertion covers all cases [1]
- rename prefix _CLOEXEC flag with _FLAG
- postpone fhold on the old file descriptor, which eliminates the need to fdrop
in error cases.
- fixup FDDUP_FCNTL check missed in the previous commit
This removes 'fp == oldfde->fde_file' assertion which had little value. kern_dup
only calls fd-related functions which cannot drop the lock or a whole lot of
races would be introduced.
Noted by: kib [1]
Modified:
head/sys/kern/kern_descrip.c
head/sys/sys/filedesc.h
Modified: head/sys/kern/kern_descrip.c
==============================================================================
--- head/sys/kern/kern_descrip.c Fri Jul 10 11:01:30 2015 (r285356)
+++ head/sys/kern/kern_descrip.c Fri Jul 10 13:54:03 2015 (r285357)
@@ -224,7 +224,6 @@ fd_last_used(struct filedesc *fdp, int s
return (-1);
}
-#ifdef INVARIANTS
static int
fdisused(struct filedesc *fdp, int fd)
{
@@ -234,7 +233,6 @@ fdisused(struct filedesc *fdp, int fd)
return ((fdp->fd_map[NDSLOT(fd)] & NDBIT(fd)) != 0);
}
-#endif
/*
* Mark a file descriptor as used.
@@ -486,7 +484,7 @@ kern_fcntl(struct thread *td, int fd, in
case F_DUPFD_CLOEXEC:
tmp = arg;
- error = kern_dup(td, FDDUP_FCNTL, FDDUP_CLOEXEC, fd, tmp);
+ error = kern_dup(td, FDDUP_FCNTL, FDDUP_FLAG_CLOEXEC, fd, tmp);
break;
case F_DUP2FD:
@@ -496,7 +494,7 @@ kern_fcntl(struct thread *td, int fd, in
case F_DUP2FD_CLOEXEC:
tmp = arg;
- error = kern_dup(td, FDDUP_FIXED, FDDUP_CLOEXEC, fd, tmp);
+ error = kern_dup(td, FDDUP_FIXED, FDDUP_FLAG_CLOEXEC, fd, tmp);
break;
case F_GETFD:
@@ -794,14 +792,13 @@ kern_dup(struct thread *td, u_int mode,
struct filedesc *fdp;
struct filedescent *oldfde, *newfde;
struct proc *p;
- struct file *fp;
struct file *delfp;
int error, maxfd;
p = td->td_proc;
fdp = p->p_fd;
- MPASS((flags & ~(FDDUP_CLOEXEC)) == 0);
+ MPASS((flags & ~(FDDUP_FLAG_CLOEXEC)) == 0);
MPASS(mode < FDDUP_LASTMODE);
/*
@@ -812,26 +809,23 @@ kern_dup(struct thread *td, u_int mode,
if (old < 0)
return (EBADF);
if (new < 0)
- return (flags & FDDUP_FCNTL ? EINVAL : EBADF);
+ return (mode == FDDUP_FCNTL ? EINVAL : EBADF);
maxfd = getmaxfd(td);
if (new >= maxfd)
- return (flags & FDDUP_FCNTL ? EINVAL : EBADF);
+ return (mode == FDDUP_FCNTL ? EINVAL : EBADF);
FILEDESC_XLOCK(fdp);
if (fget_locked(fdp, old) == NULL) {
FILEDESC_XUNLOCK(fdp);
return (EBADF);
}
- oldfde = &fdp->fd_ofiles[old];
if ((mode == FDDUP_FIXED || mode == FDDUP_MUSTREPLACE) && old == new) {
td->td_retval[0] = new;
- if (flags & FDDUP_CLOEXEC)
+ if (flags & FDDUP_FLAG_CLOEXEC)
fdp->fd_ofiles[new].fde_flags |= UF_EXCLOSE;
FILEDESC_XUNLOCK(fdp);
return (0);
}
- fp = oldfde->fde_file;
- fhold(fp);
/*
* If the caller specified a file descriptor, make sure the file
@@ -843,20 +837,15 @@ kern_dup(struct thread *td, u_int mode,
case FDDUP_FCNTL:
if ((error = fdalloc(td, new, &new)) != 0) {
FILEDESC_XUNLOCK(fdp);
- fdrop(fp, td);
return (error);
}
- newfde = &fdp->fd_ofiles[new];
break;
case FDDUP_MUSTREPLACE:
/* Target file descriptor must exist. */
- if (new >= fdp->fd_nfiles ||
- fdp->fd_ofiles[new].fde_file == NULL) {
+ if (fget_locked(fdp, new) == NULL) {
FILEDESC_XUNLOCK(fdp);
- fdrop(fp, td);
return (EBADF);
}
- newfde = &fdp->fd_ofiles[new];
break;
case FDDUP_FIXED:
if (new >= fdp->fd_nfiles) {
@@ -875,28 +864,24 @@ kern_dup(struct thread *td, u_int mode,
PROC_UNLOCK(p);
if (error != 0) {
FILEDESC_XUNLOCK(fdp);
- fdrop(fp, td);
return (EMFILE);
}
}
#endif
fdgrowtable_exp(fdp, new + 1);
- oldfde = &fdp->fd_ofiles[old];
}
- newfde = &fdp->fd_ofiles[new];
- if (newfde->fde_file == NULL)
+ if (!fdisused(fdp, new))
fdused(fdp, new);
break;
-#ifdef INVARIANTS
default:
- newfde = NULL; /* silence the compiler */
KASSERT(0, ("%s unsupported mode %d", __func__, mode));
-#endif
}
- KASSERT(fp == oldfde->fde_file, ("old fd has been modified"));
KASSERT(old != new, ("new fd is same as old"));
+ oldfde = &fdp->fd_ofiles[old];
+ fhold(oldfde->fde_file);
+ newfde = &fdp->fd_ofiles[new];
delfp = newfde->fde_file;
/*
@@ -908,7 +893,7 @@ kern_dup(struct thread *td, u_int mode,
filecaps_free(&newfde->fde_caps);
memcpy(newfde, oldfde, fde_change_size);
filecaps_copy(&oldfde->fde_caps, &newfde->fde_caps);
- if ((flags & FDDUP_CLOEXEC) != 0)
+ if ((flags & FDDUP_FLAG_CLOEXEC) != 0)
newfde->fde_flags = oldfde->fde_flags | UF_EXCLOSE;
else
newfde->fde_flags = oldfde->fde_flags & ~UF_EXCLOSE;
Modified: head/sys/sys/filedesc.h
==============================================================================
--- head/sys/sys/filedesc.h Fri Jul 10 11:01:30 2015 (r285356)
+++ head/sys/sys/filedesc.h Fri Jul 10 13:54:03 2015 (r285357)
@@ -136,7 +136,7 @@ struct filedesc_to_leader {
/* Operation types for kern_dup(). */
enum {
- FDDUP_NORMAL = 0x01, /* dup() behavior. */
+ FDDUP_NORMAL, /* dup() behavior. */
FDDUP_FCNTL, /* fcntl()-style errors. */
FDDUP_FIXED, /* Force fixed allocation. */
FDDUP_MUSTREPLACE, /* Target must exist. */
@@ -144,7 +144,7 @@ enum {
};
/* Flags for kern_dup(). */
-#define FDDUP_CLOEXEC 0x1 /* Atomically set FD_CLOEXEC. */
+#define FDDUP_FLAG_CLOEXEC 0x1 /* Atomically set UF_EXCLOSE. */
struct thread;
More information about the svn-src-all
mailing list