svn commit: r270002 - in stable/10: include lib/libc/gen

John Baldwin jhb at FreeBSD.org
Thu Aug 14 22:01:28 UTC 2014


Author: jhb
Date: Thu Aug 14 20:20:21 2014
New Revision: 270002
URL: http://svnweb.freebsd.org/changeset/base/270002

Log:
  MFC 268531,269079,269204:
  Fix various edge cases with rewinddir(), seekdir(), and telldir():
  - In the unionfs case, opendir() and fdopendir() read the directory's full
    contents and cache it.  This cache is not refreshed when rewinddir() is
    called, so rewinddir() will not notice updates to a directory.  Fix this
    by splitting the code to fetch a directory's contents out of
    __opendir_common() into a new _filldir() function and call this from
    rewinddir() when operating on a unionfs directory.
  - If rewinddir() is called on a directory opened with fdopendir() before
    any directory entries are fetched, rewinddir() will not adjust the seek
    location of the backing file descriptor.  If the file descriptor passed
    to fdopendir() had a non-zero offset, the rewinddir() will not rewind to
    the beginning.  Fix this by always seeking back to 0 in rewinddir().
    This means the dd_rewind hack can also be removed.
  - Add missing locking to rewinddir()
  - POSIX says that passing a location returned by telldir() to seekdir()
    after an intervening call to rewinddir() is undefined, so reclaim any
    pending telldir() cookies in the directory when rewinddir() is called.
  - If telldir() is called immediately after a call to seekdir(), POSIX
    requires the return value of telldir() to equal the value passed to
    seekdir().  The current seekdir code with SINGLEUSE enabled breaks
    this case as each call to telldir() allocates a new cookie.  Instead,
    remove the SINGLEUSE code and change telldir() to look for an existing
    cookie for the directory's current location rather than always creating
    a new cookie.
  
  PR:		121656

Modified:
  stable/10/include/dirent.h
  stable/10/lib/libc/gen/directory.3
  stable/10/lib/libc/gen/gen-private.h
  stable/10/lib/libc/gen/opendir.c
  stable/10/lib/libc/gen/readdir.c
  stable/10/lib/libc/gen/rewinddir.c
  stable/10/lib/libc/gen/telldir.c
  stable/10/lib/libc/gen/telldir.h
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/include/dirent.h
==============================================================================
--- stable/10/include/dirent.h	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/include/dirent.h	Thu Aug 14 20:20:21 2014	(r270002)
@@ -63,6 +63,7 @@ typedef struct _dirdesc DIR;
 #define DTF_NODUP	0x0002	/* don't return duplicate names */
 #define DTF_REWIND	0x0004	/* rewind after reading union stack */
 #define __DTF_READALL	0x0008	/* everything has been read */
+#define	__DTF_SKIPREAD	0x0010  /* assume internal buffer is populated */
 
 #else /* !__BSD_VISIBLE */
 

Modified: stable/10/lib/libc/gen/directory.3
==============================================================================
--- stable/10/lib/libc/gen/directory.3	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/directory.3	Thu Aug 14 20:20:21 2014	(r270002)
@@ -28,7 +28,7 @@
 .\"     @(#)directory.3	8.1 (Berkeley) 6/4/93
 .\" $FreeBSD$
 .\"
-.Dd August 18, 2013
+.Dd July 28, 2014
 .Dt DIRECTORY 3
 .Os
 .Sh NAME
@@ -169,6 +169,10 @@ If the directory is closed and then
 reopened, prior values returned by
 .Fn telldir
 will no longer be valid.
+Values returned by
+.Fn telldir
+are also invalidated by a call to
+.Fn rewinddir .
 .Pp
 The
 .Fn seekdir
@@ -182,13 +186,6 @@ The new position reverts to the one asso
 when the
 .Fn telldir
 operation was performed.
-State associated with the token returned by
-.Fn telldir is freed when it is passed to
-.Fn seekdir .
-If you wish return to the same location again,
-then you must create a new token with another
-.Fn telldir
-call.
 .Pp
 The
 .Fn rewinddir

Modified: stable/10/lib/libc/gen/gen-private.h
==============================================================================
--- stable/10/lib/libc/gen/gen-private.h	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/gen-private.h	Thu Aug 14 20:20:21 2014	(r270002)
@@ -48,7 +48,6 @@ struct _dirdesc {
 	char	*dd_buf;	/* data buffer */
 	int	dd_len;		/* size of data buffer */
 	long	dd_seek;	/* magic cookie returned by getdirentries */
-	long	dd_rewind;	/* magic cookie for rewinding */
 	int	dd_flags;	/* flags for readdir */
 	struct pthread_mutex	*dd_lock;	/* lock */
 	struct _telldir *dd_td;	/* telldir position recording */

Modified: stable/10/lib/libc/gen/opendir.c
==============================================================================
--- stable/10/lib/libc/gen/opendir.c	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/opendir.c	Thu Aug 14 20:20:21 2014	(r270002)
@@ -49,7 +49,7 @@ __FBSDID("$FreeBSD$");
 #include "gen-private.h"
 #include "telldir.h"
 
-static DIR * __opendir_common(int, int);
+static DIR * __opendir_common(int, int, bool);
 
 /*
  * Open a directory.
@@ -67,18 +67,10 @@ opendir(const char *name)
 DIR *
 fdopendir(int fd)
 {
-	struct stat statb;
 
-	/* Check that fd is associated with a directory. */
-	if (_fstat(fd, &statb) != 0)
-		return (NULL);
-	if (!S_ISDIR(statb.st_mode)) {
-		errno = ENOTDIR;
-		return (NULL);
-	}
 	if (_fcntl(fd, F_SETFD, FD_CLOEXEC) == -1)
 		return (NULL);
-	return (__opendir_common(fd, DTF_HIDEW|DTF_NODUP));
+	return (__opendir_common(fd, DTF_HIDEW|DTF_NODUP, true));
 }
 
 DIR *
@@ -88,11 +80,13 @@ __opendir2(const char *name, int flags)
 	DIR *dir;
 	int saved_errno;
 
+	if ((flags & (__DTF_READALL | __DTF_SKIPREAD)) != 0)
+		return (NULL);
 	if ((fd = _open(name,
 	    O_RDONLY | O_NONBLOCK | O_DIRECTORY | O_CLOEXEC)) == -1)
 		return (NULL);
 
-	dir = __opendir_common(fd, flags);
+	dir = __opendir_common(fd, flags, false);
 	if (dir == NULL) {
 		saved_errno = errno;
 		_close(fd);
@@ -110,22 +104,195 @@ opendir_compar(const void *p1, const voi
 }
 
 /*
+ * For a directory at the top of a unionfs stack, the entire directory's
+ * contents are read and cached locally until the next call to rewinddir().
+ * For the fdopendir() case, the initial seek position must be preserved.
+ * For rewinddir(), the full directory should always be re-read from the
+ * beginning.
+ *
+ * If an error occurs, the existing buffer and state of 'dirp' is left
+ * unchanged.
+ */
+bool
+_filldir(DIR *dirp, bool use_current_pos)
+{
+	struct dirent **dpv;
+	char *buf, *ddptr, *ddeptr;
+	off_t pos;
+	int fd2, incr, len, n, saved_errno, space;
+	
+	len = 0;
+	space = 0;
+	buf = NULL;
+	ddptr = NULL;
+
+	/*
+	 * Use the system page size if that is a multiple of DIRBLKSIZ.
+	 * Hopefully this can be a big win someday by allowing page
+	 * trades to user space to be done by _getdirentries().
+	 */
+	incr = getpagesize();
+	if ((incr % DIRBLKSIZ) != 0) 
+		incr = DIRBLKSIZ;
+
+	/*
+	 * The strategy here is to read all the directory
+	 * entries into a buffer, sort the buffer, and
+	 * remove duplicate entries by setting the inode
+	 * number to zero.
+	 *
+	 * We reopen the directory because _getdirentries()
+	 * on a MNT_UNION mount modifies the open directory,
+	 * making it refer to the lower directory after the
+	 * upper directory's entries are exhausted.
+	 * This would otherwise break software that uses
+	 * the directory descriptor for fchdir or *at
+	 * functions, such as fts.c.
+	 */
+	if ((fd2 = _openat(dirp->dd_fd, ".", O_RDONLY | O_CLOEXEC)) == -1)
+		return (false);
+
+	if (use_current_pos) {
+		pos = lseek(dirp->dd_fd, 0, SEEK_CUR);
+		if (pos == -1 || lseek(fd2, pos, SEEK_SET) == -1) {
+			saved_errno = errno;
+			_close(fd2);
+			errno = saved_errno;
+			return (false);
+		}
+	}
+
+	do {
+		/*
+		 * Always make at least DIRBLKSIZ bytes
+		 * available to _getdirentries
+		 */
+		if (space < DIRBLKSIZ) {
+			space += incr;
+			len += incr;
+			buf = reallocf(buf, len);
+			if (buf == NULL) {
+				saved_errno = errno;
+				_close(fd2);
+				errno = saved_errno;
+				return (false);
+			}
+			ddptr = buf + (len - space);
+		}
+
+		n = _getdirentries(fd2, ddptr, space, &dirp->dd_seek);
+		if (n > 0) {
+			ddptr += n;
+			space -= n;
+		}
+		if (n < 0) {
+			saved_errno = errno;
+			_close(fd2);
+			errno = saved_errno;
+			return (false);
+		}
+	} while (n > 0);
+	_close(fd2);
+
+	ddeptr = ddptr;
+
+	/*
+	 * There is now a buffer full of (possibly) duplicate
+	 * names.
+	 */
+	dirp->dd_buf = buf;
+
+	/*
+	 * Go round this loop twice...
+	 *
+	 * Scan through the buffer, counting entries.
+	 * On the second pass, save pointers to each one.
+	 * Then sort the pointers and remove duplicate names.
+	 */
+	for (dpv = 0;;) {
+		n = 0;
+		ddptr = buf;
+		while (ddptr < ddeptr) {
+			struct dirent *dp;
+
+			dp = (struct dirent *) ddptr;
+			if ((long)dp & 03L)
+				break;
+			if ((dp->d_reclen <= 0) ||
+			    (dp->d_reclen > (ddeptr + 1 - ddptr)))
+				break;
+			ddptr += dp->d_reclen;
+			if (dp->d_fileno) {
+				if (dpv)
+					dpv[n] = dp;
+				n++;
+			}
+		}
+
+		if (dpv) {
+			struct dirent *xp;
+
+			/*
+			 * This sort must be stable.
+			 */
+			mergesort(dpv, n, sizeof(*dpv), opendir_compar);
+
+			dpv[n] = NULL;
+			xp = NULL;
+
+			/*
+			 * Scan through the buffer in sort order,
+			 * zapping the inode number of any
+			 * duplicate names.
+			 */
+			for (n = 0; dpv[n]; n++) {
+				struct dirent *dp = dpv[n];
+
+				if ((xp == NULL) ||
+				    strcmp(dp->d_name, xp->d_name)) {
+					xp = dp;
+				} else {
+					dp->d_fileno = 0;
+				}
+				if (dp->d_type == DT_WHT &&
+				    (dirp->dd_flags & DTF_HIDEW))
+					dp->d_fileno = 0;
+			}
+
+			free(dpv);
+			break;
+		} else {
+			dpv = malloc((n+1) * sizeof(struct dirent *));
+			if (dpv == NULL)
+				break;
+		}
+	}
+
+	dirp->dd_len = len;
+	dirp->dd_size = ddptr - dirp->dd_buf;
+	return (true);
+}
+
+
+/*
  * Common routine for opendir(3), __opendir2(3) and fdopendir(3).
  */
 static DIR *
-__opendir_common(int fd, int flags)
+__opendir_common(int fd, int flags, bool use_current_pos)
 {
 	DIR *dirp;
 	int incr;
 	int saved_errno;
 	int unionstack;
-	int fd2;
-
-	fd2 = -1;
 
 	if ((dirp = malloc(sizeof(DIR) + sizeof(struct _telldir))) == NULL)
 		return (NULL);
 
+	dirp->dd_buf = NULL;
+	dirp->dd_fd = fd;
+	dirp->dd_flags = flags;
+	dirp->dd_loc = 0;
+	dirp->dd_lock = NULL;
 	dirp->dd_td = (struct _telldir *)((char *)dirp + sizeof(DIR));
 	LIST_INIT(&dirp->dd_td->td_locq);
 	dirp->dd_td->td_loccnt = 0;
@@ -154,163 +321,39 @@ __opendir_common(int fd, int flags)
 	}
 
 	if (unionstack) {
-		int len = 0;
-		int space = 0;
-		char *buf = 0;
-		char *ddptr = 0;
-		char *ddeptr;
-		int n;
-		struct dirent **dpv;
-
-		/*
-		 * The strategy here is to read all the directory
-		 * entries into a buffer, sort the buffer, and
-		 * remove duplicate entries by setting the inode
-		 * number to zero.
-		 *
-		 * We reopen the directory because _getdirentries()
-		 * on a MNT_UNION mount modifies the open directory,
-		 * making it refer to the lower directory after the
-		 * upper directory's entries are exhausted.
-		 * This would otherwise break software that uses
-		 * the directory descriptor for fchdir or *at
-		 * functions, such as fts.c.
-		 */
-		if ((fd2 = _openat(fd, ".", O_RDONLY | O_CLOEXEC)) == -1) {
-			saved_errno = errno;
-			free(buf);
-			free(dirp);
-			errno = saved_errno;
-			return (NULL);
-		}
-
-		do {
-			/*
-			 * Always make at least DIRBLKSIZ bytes
-			 * available to _getdirentries
-			 */
-			if (space < DIRBLKSIZ) {
-				space += incr;
-				len += incr;
-				buf = reallocf(buf, len);
-				if (buf == NULL)
-					goto fail;
-				ddptr = buf + (len - space);
-			}
-
-			n = _getdirentries(fd2, ddptr, space, &dirp->dd_seek);
-			if (n > 0) {
-				ddptr += n;
-				space -= n;
-			}
-		} while (n > 0);
-
-		ddeptr = ddptr;
-		flags |= __DTF_READALL;
-
-		_close(fd2);
-		fd2 = -1;
-
-		/*
-		 * There is now a buffer full of (possibly) duplicate
-		 * names.
-		 */
-		dirp->dd_buf = buf;
-
-		/*
-		 * Go round this loop twice...
-		 *
-		 * Scan through the buffer, counting entries.
-		 * On the second pass, save pointers to each one.
-		 * Then sort the pointers and remove duplicate names.
-		 */
-		for (dpv = 0;;) {
-			n = 0;
-			ddptr = buf;
-			while (ddptr < ddeptr) {
-				struct dirent *dp;
-
-				dp = (struct dirent *) ddptr;
-				if ((long)dp & 03L)
-					break;
-				if ((dp->d_reclen <= 0) ||
-				    (dp->d_reclen > (ddeptr + 1 - ddptr)))
-					break;
-				ddptr += dp->d_reclen;
-				if (dp->d_fileno) {
-					if (dpv)
-						dpv[n] = dp;
-					n++;
-				}
-			}
-
-			if (dpv) {
-				struct dirent *xp;
-
-				/*
-				 * This sort must be stable.
-				 */
-				mergesort(dpv, n, sizeof(*dpv),
-				    opendir_compar);
-
-				dpv[n] = NULL;
-				xp = NULL;
-
-				/*
-				 * Scan through the buffer in sort order,
-				 * zapping the inode number of any
-				 * duplicate names.
-				 */
-				for (n = 0; dpv[n]; n++) {
-					struct dirent *dp = dpv[n];
-
-					if ((xp == NULL) ||
-					    strcmp(dp->d_name, xp->d_name)) {
-						xp = dp;
-					} else {
-						dp->d_fileno = 0;
-					}
-					if (dp->d_type == DT_WHT &&
-					    (flags & DTF_HIDEW))
-						dp->d_fileno = 0;
-				}
-
-				free(dpv);
-				break;
-			} else {
-				dpv = malloc((n+1) * sizeof(struct dirent *));
-				if (dpv == NULL)
-					break;
-			}
-		}
-
-		dirp->dd_len = len;
-		dirp->dd_size = ddptr - dirp->dd_buf;
+		if (!_filldir(dirp, use_current_pos))
+			goto fail;
+		dirp->dd_flags |= __DTF_READALL;
 	} else {
 		dirp->dd_len = incr;
-		dirp->dd_size = 0;
 		dirp->dd_buf = malloc(dirp->dd_len);
 		if (dirp->dd_buf == NULL)
 			goto fail;
-		dirp->dd_seek = 0;
+		if (use_current_pos) {
+			/*
+			 * Read the first batch of directory entries
+			 * to prime dd_seek.  This also checks if the
+			 * fd passed to fdopendir() is a directory.
+			 */
+			dirp->dd_size = _getdirentries(dirp->dd_fd,
+			    dirp->dd_buf, dirp->dd_len, &dirp->dd_seek);
+			if (dirp->dd_size < 0) {
+				if (errno == EINVAL)
+					errno = ENOTDIR;
+				goto fail;
+			}
+			dirp->dd_flags |= __DTF_SKIPREAD;
+		} else {
+			dirp->dd_size = 0;
+			dirp->dd_seek = 0;
+		}
 	}
 
-	dirp->dd_loc = 0;
-	dirp->dd_fd = fd;
-	dirp->dd_flags = flags;
-	dirp->dd_lock = NULL;
-
-	/*
-	 * Set up seek point for rewinddir.
-	 */
-	dirp->dd_rewind = telldir(dirp);
-
 	return (dirp);
 
 fail:
 	saved_errno = errno;
-	if (fd2 != -1)
-		_close(fd2);
+	free(dirp->dd_buf);
 	free(dirp);
 	errno = saved_errno;
 	return (NULL);

Modified: stable/10/lib/libc/gen/readdir.c
==============================================================================
--- stable/10/lib/libc/gen/readdir.c	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/readdir.c	Thu Aug 14 20:20:21 2014	(r270002)
@@ -61,12 +61,14 @@ _readdir_unlocked(dirp, skip)
 				return (NULL);
 			dirp->dd_loc = 0;
 		}
-		if (dirp->dd_loc == 0 && !(dirp->dd_flags & __DTF_READALL)) {
+		if (dirp->dd_loc == 0 &&
+		    !(dirp->dd_flags & (__DTF_READALL | __DTF_SKIPREAD))) {
 			dirp->dd_size = _getdirentries(dirp->dd_fd,
 			    dirp->dd_buf, dirp->dd_len, &dirp->dd_seek);
 			if (dirp->dd_size <= 0)
 				return (NULL);
 		}
+		dirp->dd_flags &= ~__DTF_SKIPREAD;
 		dp = (struct dirent *)(dirp->dd_buf + dirp->dd_loc);
 		if ((long)dp & 03L)	/* bogus pointer check */
 			return (NULL);

Modified: stable/10/lib/libc/gen/rewinddir.c
==============================================================================
--- stable/10/lib/libc/gen/rewinddir.c	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/rewinddir.c	Thu Aug 14 20:20:21 2014	(r270002)
@@ -33,9 +33,14 @@ static char sccsid[] = "@(#)rewinddir.c	
 #include <sys/cdefs.h>
 __FBSDID("$FreeBSD$");
 
+#include "namespace.h"
 #include <sys/types.h>
 #include <dirent.h>
+#include <pthread.h>
+#include <unistd.h>
+#include "un-namespace.h"
 
+#include "libc_private.h"
 #include "gen-private.h"
 #include "telldir.h"
 
@@ -44,6 +49,16 @@ rewinddir(dirp)
 	DIR *dirp;
 {
 
-	_seekdir(dirp, dirp->dd_rewind);
-	dirp->dd_rewind = telldir(dirp);
+	if (__isthreaded)
+		_pthread_mutex_lock(&dirp->dd_lock);
+	if (dirp->dd_flags & __DTF_READALL)
+		_filldir(dirp, false);
+	else if (dirp->dd_seek != 0) {
+		(void) lseek(dirp->dd_fd, 0, SEEK_SET);
+		dirp->dd_seek = 0;
+	}
+	dirp->dd_loc = 0;
+	_reclaim_telldir(dirp);
+	if (__isthreaded)
+		_pthread_mutex_unlock(&dirp->dd_lock);
 }

Modified: stable/10/lib/libc/gen/telldir.c
==============================================================================
--- stable/10/lib/libc/gen/telldir.c	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/telldir.c	Thu Aug 14 20:20:21 2014	(r270002)
@@ -47,13 +47,6 @@ __FBSDID("$FreeBSD$");
 #include "telldir.h"
 
 /*
- * The option SINGLEUSE may be defined to say that a telldir
- * cookie may be used only once before it is freed. This option
- * is used to avoid having memory usage grow without bound.
- */
-#define SINGLEUSE
-
-/*
  * return a pointer into a directory
  */
 long
@@ -61,18 +54,31 @@ telldir(dirp)
 	DIR *dirp;
 {
 	struct ddloc *lp;
+	long idx;
 
-	if ((lp = (struct ddloc *)malloc(sizeof(struct ddloc))) == NULL)
-		return (-1);
 	if (__isthreaded)
 		_pthread_mutex_lock(&dirp->dd_lock);
-	lp->loc_index = dirp->dd_td->td_loccnt++;
-	lp->loc_seek = dirp->dd_seek;
-	lp->loc_loc = dirp->dd_loc;
-	LIST_INSERT_HEAD(&dirp->dd_td->td_locq, lp, loc_lqe);
+	LIST_FOREACH(lp, &dirp->dd_td->td_locq, loc_lqe) {
+		if (lp->loc_seek == dirp->dd_seek &&
+		    lp->loc_loc == dirp->dd_loc)
+			break;
+	}
+	if (lp == NULL) {
+		lp = malloc(sizeof(struct ddloc));
+		if (lp == NULL) {
+			if (__isthreaded)
+				_pthread_mutex_unlock(&dirp->dd_lock);
+			return (-1);
+		}
+		lp->loc_index = dirp->dd_td->td_loccnt++;
+		lp->loc_seek = dirp->dd_seek;
+		lp->loc_loc = dirp->dd_loc;
+		LIST_INSERT_HEAD(&dirp->dd_td->td_locq, lp, loc_lqe);
+	}
+	idx = lp->loc_index;
 	if (__isthreaded)
 		_pthread_mutex_unlock(&dirp->dd_lock);
-	return (lp->loc_index);
+	return (idx);
 }
 
 /*
@@ -94,7 +100,7 @@ _seekdir(dirp, loc)
 	if (lp == NULL)
 		return;
 	if (lp->loc_loc == dirp->dd_loc && lp->loc_seek == dirp->dd_seek)
-		goto found;
+		return;
 	(void) lseek(dirp->dd_fd, (off_t)lp->loc_seek, SEEK_SET);
 	dirp->dd_seek = lp->loc_seek;
 	dirp->dd_loc = 0;
@@ -103,11 +109,6 @@ _seekdir(dirp, loc)
 		if (dp == NULL)
 			break;
 	}
-found:
-#ifdef SINGLEUSE
-	LIST_REMOVE(lp, loc_lqe);
-	free((caddr_t)lp);
-#endif
 }
 
 /*

Modified: stable/10/lib/libc/gen/telldir.h
==============================================================================
--- stable/10/lib/libc/gen/telldir.h	Thu Aug 14 20:17:23 2014	(r270001)
+++ stable/10/lib/libc/gen/telldir.h	Thu Aug 14 20:20:21 2014	(r270002)
@@ -36,6 +36,7 @@
 #define	_TELLDIR_H_
 
 #include <sys/queue.h>
+#include <stdbool.h>
 
 /*
  * One of these structures is malloced to describe the current directory
@@ -59,6 +60,7 @@ struct _telldir {
 	long	td_loccnt;	/* index of entry for sequential readdir's */
 };
 
+bool		_filldir(DIR *, bool);
 struct dirent	*_readdir_unlocked(DIR *, int);
 void 		_reclaim_telldir(DIR *);
 void 		_seekdir(DIR *, long);


More information about the svn-src-all mailing list