Hi,
after some more testing and proofreading, i found some issues
with my libc/gen patch; please delete what i sent yesterday.
Here is an updated patch which now works correctly with Otto's
regression test, with the new test i just committed, and with
the test from the Perl test suite Andrew pointed out, even with
threads enabled. It also survived quite some manual testing.
Changes with respect to yesterday:
* Unfortunately, dirent.d_off is the offset of the *next*
entry with respect to the beginning of the directory,
not of the *current* entry. So, we simply don't know
where the first entry in dd_buf is located in the directory,
so we cannot seek back to it without calling getdents() again.
Thus, we can as well continue using dd_loc = 0 as the flag
to invalidate dd_buf, no use changing it to -1.
* Due to the same misunderstanding of d_off, i introduced an
off-by-one error, sometimes backing up one entry too far.
Ouch.
* Initialize dd_size in opendir() and test it in seekdir()
to make sure that we don't access dd_buf uninitialized.
Comments? Tests? OKs?
Ingo
P.S.
Oh, and as requested by Theo, i'm now fixing the bug 90 years
earlier than i originally intended.
Index: opendir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/opendir.c,v
retrieving revision 1.25
diff -u -p -r1.25 opendir.c
--- opendir.c 6 Oct 2013 17:57:11 -0000 1.25
+++ opendir.c 3 Nov 2013 00:23:09 -0000
@@ -111,6 +111,7 @@ __fdopendir(int fd)
return (NULL);
}
+ dirp->dd_size = 0;
dirp->dd_loc = 0;
dirp->dd_fd = fd;
dirp->dd_lock = NULL;
Index: rewinddir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/rewinddir.c,v
retrieving revision 1.10
diff -u -p -r1.10 rewinddir.c
--- rewinddir.c 13 Aug 2013 05:52:12 -0000 1.10
+++ rewinddir.c 3 Nov 2013 00:23:09 -0000
@@ -1,5 +1,5 @@
/* $OpenBSD: rewinddir.c,v 1.10 2013/08/13 05:52:12 guenther Exp $ */
-/*-
+/*
* Copyright (c) 1990, 1993
* The Regents of the University of California. All rights reserved.
*
@@ -28,16 +28,12 @@
* SUCH DAMAGE.
*/
-#include <sys/types.h>
#include <dirent.h>
-
#include "thread_private.h"
#include "telldir.h"
void
rewinddir(DIR *dirp)
{
- _MUTEX_LOCK(&dirp->dd_lock);
- __seekdir(dirp, 0);
- _MUTEX_UNLOCK(&dirp->dd_lock);
+ seekdir(dirp, 0);
}
Index: seekdir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.9
diff -u -p -r1.9 seekdir.c
--- seekdir.c 5 Jun 2007 18:11:48 -0000 1.9
+++ seekdir.c 3 Nov 2013 00:23:09 -0000
@@ -1,46 +1,71 @@
/* $OpenBSD: seekdir.c,v 1.9 2007/06/05 18:11:48 kurt Exp $ */
/*
- * Copyright (c) 1983, 1993
- * The Regents of the University of California. All rights reserved.
+ * Copyright (c) 2013 Ingo Schwarze <schwa...@openbsd.org>
*
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
*
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
-#include <sys/param.h>
#include <dirent.h>
+#include <unistd.h>
+
#include "thread_private.h"
#include "telldir.h"
/*
* Seek to an entry in a directory.
- * __seekdir is in telldir.c so that it can share opaque data structures.
+ * Only values returned by "telldir" should be passed to seekdir.
*/
void
seekdir(DIR *dirp, long loc)
{
+ struct dirent *dp;
+
+ /*
+ * First check whether the directory entry to seek for
+ * is still buffered in the directory structure in memory.
+ */
+
_MUTEX_LOCK(&dirp->dd_lock);
- __seekdir(dirp, loc);
+ dp = (struct dirent *)dirp->dd_buf;
+ if (dirp->dd_size > 0 && dp->d_off <= loc) {
+ for (dirp->dd_loc = 0;
+ dirp->dd_loc < dirp->dd_size;
+ dirp->dd_loc += dp->d_reclen) {
+ dp = (struct dirent *)(dirp->dd_buf + dirp->dd_loc);
+ if (dp->d_off < loc)
+ continue;
+
+ /*
+ * Entry found in the buffer, use it.
+ * If readdir(3) follows, this will
+ * save us a getdents(2) syscall.
+ * Note that d_off is the offset of
+ * the _next_ entry, so advance dd_loc.
+ */
+
+ dirp->dd_curpos = loc;
+ dirp->dd_loc += dp->d_reclen;
+ _MUTEX_UNLOCK(&dirp->dd_lock);
+ return;
+ }
+ }
+
+ /*
+ * The entry is not in the buffer, prepare a call to getdents(2).
+ * In particular, invalidate dd_loc.
+ */
+
+ dirp->dd_loc = 0;
+ dirp->dd_curpos = lseek(dirp->dd_fd, loc, SEEK_SET);
_MUTEX_UNLOCK(&dirp->dd_lock);
}
Index: telldir.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/telldir.c,v
retrieving revision 1.15
diff -u -p -r1.15 telldir.c
--- telldir.c 16 Aug 2013 05:27:39 -0000 1.15
+++ telldir.c 3 Nov 2013 00:23:09 -0000
@@ -28,44 +28,21 @@
* SUCH DAMAGE.
*/
-#include <sys/param.h>
-#include <sys/queue.h>
#include <dirent.h>
-#include <stdlib.h>
-#include <unistd.h>
-
#include "thread_private.h"
#include "telldir.h"
-int _readdir_unlocked(DIR *, struct dirent **, int);
-
/*
* return a pointer into a directory
*/
long
-_telldir_unlocked(DIR *dirp)
-{
- return (dirp->dd_curpos);
-}
-
-long
telldir(DIR *dirp)
{
long i;
_MUTEX_LOCK(&dirp->dd_lock);
- i = _telldir_unlocked(dirp);
+ i = dirp->dd_curpos;
_MUTEX_UNLOCK(&dirp->dd_lock);
return (i);
-}
-
-/*
- * seek to an entry in a directory.
- * Only values returned by "telldir" should be passed to seekdir.
- */
-void
-__seekdir(DIR *dirp, long loc)
-{
- dirp->dd_curpos = lseek(dirp->dd_fd, loc, SEEK_SET);
}
Index: telldir.h
===================================================================
RCS file: /cvs/src/lib/libc/gen/telldir.h,v
retrieving revision 1.6
diff -u -p -r1.6 telldir.h
--- telldir.h 13 Aug 2013 05:52:13 -0000 1.6
+++ telldir.h 3 Nov 2013 00:23:09 -0000
@@ -47,7 +47,4 @@ struct _dirdesc {
void *dd_lock; /* mutex to protect struct */
};
-long _telldir_unlocked(DIR *);
-void __seekdir(DIR *, long);
-
#endif
