Subject: Re: lib/24324: telldir issues
To: None <lib-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Christos Zoulas <christos@zoulas.com>
List: netbsd-bugs
Date: 05/17/2006 15:45:02
The following reply was made to PR lib/24324; it has been noted by GNATS.
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@netbsd.org, lib-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
arnej@europe.yahoo-inc.com
Cc:
Subject: Re: lib/24324: telldir issues
Date: Wed, 17 May 2006 11:41:30 -0400
On May 13, 5:45pm, david@l8s.co.uk (David Laight) wrote:
-- Subject: Re: lib/24324: telldir issues
| Thinking further, replacing the 'long dd_rewind' with a pointer to
| library private data would be even better. This would let the
| library to hide the gory details from applications.
| Since this woule be allocated by opendir(), telldir() could keep
| it's 'const DIR *' prototype.
|
| For complete binary compatibility, the value in the (old) dd_rewind
| field could be accepted as a token for 'start of directory'.
How about this?
christos
Index: include/dirent.h
===================================================================
RCS file: /cvsroot/src/include/dirent.h,v
retrieving revision 1.28
diff -u -r1.28 dirent.h
--- include/dirent.h 26 Mar 2006 18:22:40 -0000 1.28
+++ include/dirent.h 17 May 2006 15:38:41 -0000
@@ -62,7 +62,7 @@
char *dd_buf; /* data buffer */
int dd_len; /* size of data buffer */
off_t dd_seek; /* magic cookie returned by getdents */
- long dd_rewind; /* magic cookie for rewinding */
+ void *dd_internal; /* state for seekdir/telldir */
int dd_flags; /* flags for readdir */
void *dd_lock; /* lock for concurrent access */
};
@@ -94,7 +94,7 @@
#endif
#if defined(_XOPEN_SOURCE) || defined(_NETBSD_SOURCE)
void seekdir(DIR *, long);
-long telldir(const DIR *);
+long telldir(DIR *);
#endif /* defined(_NETBSD_SOURCE) || defined(_XOPEN_SOURCE) */
#if defined(_NETBSD_SOURCE)
#ifndef __LIBC12_SOURCE__
Index: lib/libc/gen/closedir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/closedir.c,v
retrieving revision 1.14
diff -u -r1.14 closedir.c
--- lib/libc/gen/closedir.c 24 Jan 2006 19:33:10 -0000 1.14
+++ lib/libc/gen/closedir.c 17 May 2006 15:38:41 -0000
@@ -49,6 +49,8 @@
#include <stdlib.h>
#include <unistd.h>
+#include "dirent_private.h"
+
#ifdef __weak_alias
__weak_alias(closedir,_closedir)
#endif
@@ -57,10 +59,10 @@
* close a directory.
*/
int
-closedir(dirp)
- DIR *dirp;
+closedir(DIR *dirp)
{
int fd;
+ struct dirpos *poslist;
_DIAGASSERT(dirp != NULL);
@@ -68,11 +70,18 @@
if (__isthreaded)
mutex_lock((mutex_t *)dirp->dd_lock);
#endif
- _seekdir_unlocked(dirp, dirp->dd_rewind);/* free seekdir storage */
fd = dirp->dd_fd;
dirp->dd_fd = -1;
dirp->dd_loc = 0;
- free((void *)dirp->dd_buf);
+ free(dirp->dd_buf);
+
+ /* free seekdir/telldir storage */
+ for (poslist = dirp->dd_internal; poslist; ) {
+ struct dirpos *nextpos = poslist->dp_next;
+ free(poslist);
+ poslist = nextpos;
+ }
+
#ifdef _REENTRANT
if (__isthreaded) {
mutex_unlock((mutex_t *)dirp->dd_lock);
Index: lib/libc/gen/directory.3
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/directory.3,v
retrieving revision 1.24
diff -u -r1.24 directory.3
--- lib/libc/gen/directory.3 7 Aug 2003 16:42:47 -0000 1.24
+++ lib/libc/gen/directory.3 17 May 2006 15:38:41 -0000
@@ -29,7 +29,7 @@
.\"
.\" @(#)directory.3 8.1 (Berkeley) 6/4/93
.\"
-.Dd May 28, 2003
+.Dd May 16, 2006
.Dt DIRECTORY 3
.Os
.Sh NAME
@@ -53,7 +53,7 @@
.Ft int
.Fn readdir_r "DIR * restrict dirp" "struct dirent * restrict entry" "struct dirent ** restrict result"
.Ft long
-.Fn telldir "const DIR *dirp"
+.Fn telldir "DIR *dirp"
.Ft void
.Fn seekdir "DIR *dirp" "long loc"
.Ft void
Index: lib/libc/gen/opendir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/opendir.c,v
retrieving revision 1.30
diff -u -r1.30 opendir.c
--- lib/libc/gen/opendir.c 24 Jan 2006 19:33:10 -0000 1.30
+++ lib/libc/gen/opendir.c 17 May 2006 15:38:41 -0000
@@ -53,12 +53,13 @@
#include <string.h>
#include <unistd.h>
+#include "dirent_private.h"
+
/*
* Open a directory.
*/
DIR *
-opendir(name)
- const char *name;
+opendir(const char *name)
{
_DIAGASSERT(name != NULL);
@@ -67,9 +68,7 @@
}
DIR *
-__opendir2(name, flags)
- const char *name;
- int flags;
+__opendir2(const char *name, int flags)
{
DIR *dirp = NULL;
int fd;
@@ -304,7 +303,8 @@
mutex_init((mutex_t *)dirp->dd_lock, NULL);
}
#endif
- dirp->dd_rewind = _telldir_unlocked(dirp);
+ dirp->dd_internal = NULL;
+ (void)_telldir_unlocked(dirp);
return (dirp);
error:
serrno = errno;
Index: lib/libc/gen/readdir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/readdir.c,v
retrieving revision 1.22
diff -u -r1.22 readdir.c
--- lib/libc/gen/readdir.c 24 Jan 2006 19:33:10 -0000 1.22
+++ lib/libc/gen/readdir.c 17 May 2006 15:38:42 -0000
@@ -48,6 +48,8 @@
#include <string.h>
#include <unistd.h>
+#include "dirent_private.h"
+
/*
* get next entry in a directory.
*/
Index: lib/libc/gen/rewinddir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/rewinddir.c,v
retrieving revision 1.11
diff -u -r1.11 rewinddir.c
--- lib/libc/gen/rewinddir.c 24 Jan 2006 19:33:10 -0000 1.11
+++ lib/libc/gen/rewinddir.c 17 May 2006 15:38:42 -0000
@@ -45,25 +45,27 @@
#include <dirent.h>
+#include "dirent_private.h"
+
#ifdef __weak_alias
__weak_alias(rewinddir,_rewinddir)
#endif
void
-rewinddir(dirp)
- DIR *dirp;
+rewinddir(DIR *dirp)
{
+ struct dirpos *dp = dirp->dd_internal;
+
+ while (dp->dp_next)
+ dp = dp->dp_next;
#ifdef _REENTRANT
if (__isthreaded) {
mutex_lock((mutex_t *)dirp->dd_lock);
- _seekdir_unlocked(dirp, dirp->dd_rewind);
- dirp->dd_rewind = _telldir_unlocked(dirp);
+ _seekdir_unlocked(dirp, (long)(intptr_t)dp);
mutex_unlock((mutex_t *)dirp->dd_lock);
return;
}
#endif
- _seekdir_unlocked(dirp, dirp->dd_rewind);
- dirp->dd_rewind = _telldir_unlocked(dirp);
-
+ _seekdir_unlocked(dirp, (long)(intptr_t)dp);
}
Index: lib/libc/gen/seekdir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/seekdir.c,v
retrieving revision 1.13
diff -u -r1.13 seekdir.c
--- lib/libc/gen/seekdir.c 24 Jan 2006 19:33:10 -0000 1.13
+++ lib/libc/gen/seekdir.c 17 May 2006 15:38:42 -0000
@@ -45,6 +45,8 @@
#include <dirent.h>
+#include "dirent_private.h"
+
#ifdef __weak_alias
__weak_alias(seekdir,_seekdir)
#endif
@@ -54,9 +56,7 @@
* _seekdir_unlocked is in telldir.c so that it can share opaque data structures.
*/
void
-seekdir(dirp, loc)
- DIR *dirp;
- long loc;
+seekdir(DIR *dirp, long loc)
{
#ifdef _REENTRANT
Index: lib/libc/gen/telldir.c
===================================================================
RCS file: /cvsroot/src/lib/libc/gen/telldir.c,v
retrieving revision 1.17
diff -u -r1.17 telldir.c
--- lib/libc/gen/telldir.c 24 Jan 2006 19:33:10 -0000 1.17
+++ lib/libc/gen/telldir.c 17 May 2006 15:38:42 -0000
@@ -48,48 +48,24 @@
#include <stdlib.h>
#include <unistd.h>
+#include "dirent_private.h"
+
#ifdef __weak_alias
__weak_alias(telldir,_telldir)
#endif
-/*
- * 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
-
-/*
- * One of these structures is malloced to describe the current directory
- * position each time telldir is called. It records the current magic
- * cookie returned by getdirentries and the offset within the buffer
- * associated with that return value.
- */
-struct ddloc {
- struct ddloc *loc_next;/* next structure in list */
- long loc_index; /* key associated with structure */
- off_t loc_seek; /* magic cookie returned by getdirentries */
- long loc_loc; /* offset of entry in buffer */
-};
-
-#define NDIRHASH 32 /* Num of hash lists, must be a power of 2 */
-#define LOCHASH(i) (((int)i)&(NDIRHASH-1))
-
-static long dd_loccnt; /* Index of entry for sequential readdir's */
-static struct ddloc *dd_hash[NDIRHASH]; /* Hash list heads for ddlocs */
-
long
-telldir(const DIR *dirp)
+telldir(DIR *dirp)
{
long rv;
#ifdef _REENTRANT
if (__isthreaded) {
mutex_lock((mutex_t *)dirp->dd_lock);
- rv = _telldir_unlocked(dirp);
+ rv = (intptr_t)_telldir_unlocked(dirp);
mutex_unlock((mutex_t *)dirp->dd_lock);
} else
#endif
- rv = _telldir_unlocked(dirp);
+ rv = (intptr_t)_telldir_unlocked(dirp);
return rv;
}
@@ -97,20 +73,24 @@
* return a pointer into a directory
*/
long
-_telldir_unlocked(const DIR *dirp)
+_telldir_unlocked(DIR *dirp)
{
- long idx;
- struct ddloc *lp;
+ struct dirpos *lp;
- if ((lp = (struct ddloc *)malloc(sizeof(struct ddloc))) == NULL)
+ for (lp = dirp->dd_internal; lp; lp = lp->dp_next)
+ if (lp->dp_seek == dirp->dd_seek &&
+ lp->dp_loc == dirp->dd_loc)
+ return (intptr_t)lp;
+
+ if ((lp = malloc(sizeof(*lp))) == NULL)
return (-1);
- idx = dd_loccnt++;
- lp->loc_index = idx;
- lp->loc_seek = dirp->dd_seek;
- lp->loc_loc = dirp->dd_loc;
- lp->loc_next = dd_hash[LOCHASH(idx)];
- dd_hash[LOCHASH(idx)] = lp;
- return (idx);
+
+ lp->dp_seek = dirp->dd_seek;
+ lp->dp_loc = dirp->dd_loc;
+ lp->dp_next = dirp->dd_internal;
+ dirp->dd_internal = lp;
+
+ return (intptr_t)lp;
}
/*
@@ -120,35 +100,23 @@
void
_seekdir_unlocked(DIR *dirp, long loc)
{
- struct ddloc *lp;
- struct ddloc **prevlp;
- struct dirent *dp;
+ struct dirpos *lp;
_DIAGASSERT(dirp != NULL);
- prevlp = &dd_hash[LOCHASH(loc)];
- lp = *prevlp;
- while (lp != NULL) {
- if (lp->loc_index == loc)
+ for (lp = dirp->dd_internal; lp; lp = lp->dp_next)
+ if ((intptr_t)lp == loc)
break;
- prevlp = &lp->loc_next;
- lp = lp->loc_next;
- }
+
if (lp == NULL)
return;
- if (lp->loc_loc == dirp->dd_loc && lp->loc_seek == dirp->dd_seek)
- goto found;
- (void) lseek(dirp->dd_fd, (off_t)lp->loc_seek, SEEK_SET);
- dirp->dd_seek = lp->loc_seek;
+
+ if (lp->dp_loc == dirp->dd_loc && lp->dp_seek == dirp->dd_seek)
+ return;
+
+ dirp->dd_seek = lseek(dirp->dd_fd, lp->dp_seek, SEEK_SET);
dirp->dd_loc = 0;
- while (dirp->dd_loc < lp->loc_loc) {
- dp = _readdir_unlocked(dirp);
- if (dp == NULL)
+ while (dirp->dd_loc < lp->dp_loc)
+ if (_readdir_unlocked(dirp) == NULL)
break;
- }
-found:
-#ifdef SINGLEUSE
- *prevlp = lp->loc_next;
- free(lp);
-#endif
}
Index: lib/libc/include/extern.h
===================================================================
RCS file: /cvsroot/src/lib/libc/include/extern.h,v
retrieving revision 1.12
diff -u -r1.12 extern.h
--- lib/libc/include/extern.h 24 Feb 2006 19:33:09 -0000 1.12
+++ lib/libc/include/extern.h 17 May 2006 15:38:42 -0000
@@ -46,14 +46,6 @@
int __sigaction_sigtramp __P((int, const struct sigaction *,
struct sigaction *, const void *, int));
-struct _dirdesc;
-void _seekdir_unlocked(struct _dirdesc *, long);
-long _telldir_unlocked(const struct _dirdesc *);
-#ifndef __LIBC12_SOURCE__
-struct dirent;
-struct dirent *_readdir_unlocked(struct _dirdesc *) __RENAME(___readdir_unlocked30);
-#endif
-
#ifdef WIDE_DOUBLE
char *__hdtoa(double, const char *, int, int *, int *, char **);
char *__hldtoa(long double, const char *, int, int *, int *, char **);
--- /dev/null 2006-05-17 11:38:40.000000000 -0400
+++ lib/libc/gen/dirent_private.h 2006-05-16 22:31:32.000000000 -0400
@@ -0,0 +1,22 @@
+/* $NetBSD: pw_private.h,v 1.2 2003/07/26 19:24:43 salo Exp $ */
+
+/*
+ * One struct _dirpos is malloced to describe the current directory
+ * position each time telldir is called. It records the current magic
+ * cookie returned by getdirentries and the offset within the buffer
+ * associated with that return value.
+ */
+struct dirpos {
+ struct dirpos *dp_next; /* next structure in list */
+ off_t dp_seek; /* magic cookie returned by getdirentries */
+ long dp_loc; /* offset of entry in buffer */
+};
+
+struct _dirdesc;
+void _seekdir_unlocked(struct _dirdesc *, long);
+long _telldir_unlocked(struct _dirdesc *);
+#ifndef __LIBC12_SOURCE__
+struct dirent;
+struct dirent *_readdir_unlocked(struct _dirdesc *)
+ __RENAME(___readdir_unlocked30);
+#endif