Date: Thu, 13 Mar 2008 01:01:18 GMT From: Gary Stanley <gary@velocity-servers.net> To: freebsd-gnats-submit@FreeBSD.org Subject: i386/121656: [libc] [PATCH] telldir issues Message-ID: <200803130101.m2D11IRk024940@www.freebsd.org> Resent-Message-ID: <200803130110.m2D1A1gY008785@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 121656
>Category: i386
>Synopsis: [libc] [PATCH] telldir issues
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-i386
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Thu Mar 13 01:10:01 UTC 2008
>Closed-Date:
>Last-Modified:
>Originator: Gary Stanley
>Release: 7.0-RELEASE
>Organization:
>Environment:
>Description:
After investigating some issues with some programs here, I've figured out what the cause is. It seems there is a bug with libc's telldir/seekdir. I was able to track a fix down that originated in NetBSD, so I've taken the time to port it. I've tested it with some regression tools which I will attach to this PR. If you chose to commit (any) parts of this diff you will probably need to bump the OS version because the size of dirdesc will change. I didn't notice any issues with this at all, so you probably will want to avoid doing that.
>How-To-Repeat:
Here's some of the regression test program that show the problem. If you compile seekdir-twice.c with a simple "#define DEBUG" you'll get more useful info.
Busted telldir/seekdir machine (RELENG_7)
first found: a
pos returned by telldir: 1
second found: b
third found: c
should be == second: b
pos given to seekdir: 1
pos returned by telldir: 2
should be == second: c
should be == second: c
Fixed seekdir/telldir with included patch:
first found: a
pos returned by telldir: 673198160
second found: b
third found: c
should be == second: b
pos given to seekdir: 673198160
pos returned by telldir: 673198160
should be == second: b
should be == second: b
>Fix:
Apply libc.diff to RELENG_7/RELENG_6 and rereun regression test tools. Everything appears to pass and I've been using these on a few machine for the past couple of months.
Patch attached with submission follows:
# This is a shell archive. Save it in a file, remove anything before
# this line, and then unpack it by entering "sh file". Note, it may
# create directories; files and directories will be owned by you and
# have default permissions.
#
# This archive contains:
#
# .
# ./telldir-memuse.c
# ./libc.diff
# ./seekdir-twice.c
#
echo c - .
mkdir -p . > /dev/null 2>&1
echo x - ./telldir-memuse.c
sed 's/^X//' >./telldir-memuse.c << 'END-of-./telldir-memuse.c'
X#include <stdlib.h>
X#include <unistd.h>
X#include <dirent.h>
X#include <err.h>
X
X
Xint main(void) {
X DIR *dp;
X long loc;
X void *memused;
X int i;
X int oktouse = 4096;
X
X dp = opendir(".");
X if (dp == NULL)
X err(EXIT_FAILURE, "Could not open current directory");
X loc = telldir(dp);
X memused = sbrk(0);
X closedir(dp);
X
X for (i=0; i<1000; i++) {
X dp = opendir(".");
X if (dp == NULL)
X err(EXIT_FAILURE, "Could not open current directory");
X loc = telldir(dp);
X closedir(dp);
X
X if (sbrk(0) - memused > oktouse) {
X warnx("Used %d extra bytes for %d telldir calls",
X (sbrk(0)-memused), i);
X oktouse = sbrk(0) - memused;
X }
X }
X if (oktouse > 4096) {
X errx(EXIT_FAILURE, "Failure: leaked %d bytes", oktouse);
X } else {
X printf("OK: used %d bytes\n", sbrk(0)-memused);
X }
X return 0;
X}
END-of-./telldir-memuse.c
echo x - ./libc.diff
sed 's/^X//' >./libc.diff << 'END-of-./libc.diff'
X--- include/dirent.h.old 2003-12-07 15:10:06.000000000 -0600
X+++ include/dirent.h 2008-03-12 19:31:09.000000000 -0500
X@@ -69,7 +69,7 @@
X char *dd_buf; /* data buffer */
X int dd_len; /* size of data buffer */
X long dd_seek; /* magic cookie returned by getdirentries */
X- long dd_rewind; /* magic cookie for rewinding */
X+ void *dd_internal; /* state for seekdir/telldir */
X int dd_flags; /* flags for readdir */
X void *dd_lock; /* hack to avoid including <pthread.h> */
X struct _telldir *dd_td; /* telldir position recording */
Xdiff -u -r -N lib/libc.old/gen/closedir.c lib/libc/gen/closedir.c
X--- lib/libc.old/gen/closedir.c 2007-01-08 18:27:53.000000000 -0600
X+++ lib/libc/gen/closedir.c 2008-03-12 19:22:10.000000000 -0500
X@@ -43,24 +43,31 @@
X
X #include "libc_private.h"
X #include "telldir.h"
X+#include "dirent_private.h"
X
X /*
X * close a directory.
X */
X int
X-closedir(dirp)
X- DIR *dirp;
X+closedir(DIR *dirp)
X {
X int fd;
X+ struct dirpos *poslist;
X
X if (__isthreaded)
X _pthread_mutex_lock((pthread_mutex_t *)&dirp->dd_lock);
X- _seekdir(dirp, dirp->dd_rewind); /* free seekdir storage */
X fd = dirp->dd_fd;
X dirp->dd_fd = -1;
X dirp->dd_loc = 0;
X- free((void *)dirp->dd_buf);
X- _reclaim_telldir(dirp);
X+ free(dirp->dd_buf);
X+
X+ /* free seekdir/telldir storage */
X+ for (poslist = dirp->dd_internal; poslist; ) {
X+ struct dirpos *nextpos = poslist->dp_next;
X+ free(poslist);
X+ poslist = nextpos;
X+ }
X+
X if (__isthreaded) {
X _pthread_mutex_unlock((pthread_mutex_t *)&dirp->dd_lock);
X _pthread_mutex_destroy((pthread_mutex_t *)&dirp->dd_lock);
Xdiff -u -r -N lib/libc.old/gen/dirent_private.h lib/libc/gen/dirent_private.h
X--- lib/libc.old/gen/dirent_private.h 1969-12-31 18:00:00.000000000 -0600
X+++ lib/libc/gen/dirent_private.h 2008-03-12 19:22:10.000000000 -0500
X@@ -0,0 +1,17 @@
X+/* $NetBSD: dirent_private.h,v 1.1 2006/05/17 20:36:50 christos Exp $ */
X+
X+/*
X+ * One struct _dirpos is malloced to describe the current directory
X+ * position each time telldir is called. It records the current magic
X+ * cookie returned by getdirentries and the offset within the buffer
X+ * associated with that return value.
X+ */
X+struct dirpos {
X+ struct dirpos *dp_next; /* next structure in list */
X+ off_t dp_seek; /* magic cookie returned by getdirentries */
X+ long dp_loc; /* offset of entry in buffer */
X+};
X+
X+struct _dirdesc;
X+void _seekdir_unlocked(struct _dirdesc *, long);
X+long _telldir_unlocked(struct _dirdesc *);
Xdiff -u -r -N lib/libc.old/gen/opendir.c lib/libc/gen/opendir.c
X--- lib/libc.old/gen/opendir.c 2007-01-08 18:27:54.000000000 -0600
X+++ lib/libc/gen/opendir.c 2008-03-12 19:22:10.000000000 -0500
X@@ -47,21 +47,20 @@
X #include "un-namespace.h"
X
X #include "telldir.h"
X+#include "dirent_private.h"
X+
X /*
X * Open a directory.
X */
X DIR *
X-opendir(name)
X- const char *name;
X+opendir(const char *name)
X {
X
X return (__opendir2(name, DTF_HIDEW|DTF_NODUP));
X }
X
X DIR *
X-__opendir2(name, flags)
X- const char *name;
X- int flags;
X+__opendir2(const char *name, int flags)
X {
X DIR *dirp;
X int fd;
X@@ -270,7 +269,8 @@
X /*
X * Set up seek point for rewinddir.
X */
X- dirp->dd_rewind = telldir(dirp);
X+ dirp->dd_internal = NULL;
X+ (void)telldir(dirp);
X
X return (dirp);
X
Xdiff -u -r -N lib/libc.old/gen/readdir.c lib/libc/gen/readdir.c
X--- lib/libc.old/gen/readdir.c 2007-01-08 18:27:55.000000000 -0600
X+++ lib/libc/gen/readdir.c 2008-03-12 19:22:10.000000000 -0500
X@@ -43,6 +43,7 @@
X
X #include "libc_private.h"
X #include "telldir.h"
X+#include "dirent_private.h"
X
X /*
X * get next entry in a directory.
Xdiff -u -r -N lib/libc.old/gen/rewinddir.c lib/libc/gen/rewinddir.c
X--- lib/libc.old/gen/rewinddir.c 2007-01-08 18:27:55.000000000 -0600
X+++ lib/libc/gen/rewinddir.c 2008-03-12 19:22:10.000000000 -0500
X@@ -37,12 +37,16 @@
X #include <dirent.h>
X
X #include "telldir.h"
X+#include "dirent_private.h"
X
X void
X-rewinddir(dirp)
X- DIR *dirp;
X+rewinddir(DIR *dirp)
X {
X
X- _seekdir(dirp, dirp->dd_rewind);
X- dirp->dd_rewind = telldir(dirp);
X+ struct dirpos *dp = dirp->dd_internal;
X+
X+ while (dp->dp_next)
X+ dp = dp->dp_next;
X+
X+ _seekdir(dirp, (long)(intptr_t)dp);
X }
Xdiff -u -r -N lib/libc.old/gen/seekdir.c lib/libc/gen/seekdir.c
X--- lib/libc.old/gen/seekdir.c 2007-01-08 18:27:55.000000000 -0600
X+++ lib/libc/gen/seekdir.c 2008-03-12 19:22:10.000000000 -0500
X@@ -41,15 +41,14 @@
X
X #include "libc_private.h"
X #include "telldir.h"
X+#include "dirent_private.h"
X
X /*
X * Seek to an entry in a directory.
X * _seekdir is in telldir.c so that it can share opaque data structures.
X */
X void
X-seekdir(dirp, loc)
X- DIR *dirp;
X- long loc;
X+seekdir(DIR *dirp, long loc)
X {
X if (__isthreaded)
X _pthread_mutex_lock((pthread_mutex_t *)&dirp->dd_lock);
Xdiff -u -r -N lib/libc.old/gen/telldir.c lib/libc/gen/telldir.c
X--- lib/libc.old/gen/telldir.c 2007-01-08 18:27:55.000000000 -0600
X+++ lib/libc/gen/telldir.c 2008-03-12 19:22:56.000000000 -0500
X@@ -45,33 +45,30 @@
X #include "libc_private.h"
X #include "telldir.h"
X
X-/*
X- * The option SINGLEUSE may be defined to say that a telldir
X- * cookie may be used only once before it is freed. This option
X- * is used to avoid having memory usage grow without bound.
X- */
X-#define SINGLEUSE
X+#include "dirent_private.h"
X
X /*
X * return a pointer into a directory
X */
X long
X-telldir(dirp)
X- DIR *dirp;
X+telldir(DIR *dirp)
X {
X- struct ddloc *lp;
X+ struct dirpos *lp;
X
X- if ((lp = (struct ddloc *)malloc(sizeof(struct ddloc))) == NULL)
X+ for (lp = dirp->dd_internal; lp; lp = lp->dp_next)
X+ if (lp->dp_seek == dirp->dd_seek &&
X+ lp->dp_loc == dirp->dd_loc)
X+ return (intptr_t)lp;
X+ if ((lp = malloc(sizeof(*lp))) == NULL)
X return (-1);
X- if (__isthreaded)
X- _pthread_mutex_lock((pthread_mutex_t *)&dirp->dd_lock);
X- lp->loc_index = dirp->dd_td->td_loccnt++;
X- lp->loc_seek = dirp->dd_seek;
X- lp->loc_loc = dirp->dd_loc;
X- LIST_INSERT_HEAD(&dirp->dd_td->td_locq, lp, loc_lqe);
X- if (__isthreaded)
X- _pthread_mutex_unlock((pthread_mutex_t *)&dirp->dd_lock);
X- return (lp->loc_index);
X+
X+ lp->dp_seek = dirp->dd_seek;
X+ lp->dp_loc = dirp->dd_loc;
X+ lp->dp_next = dirp->dd_internal;
X+ dirp->dd_internal = lp;
X+
X+ return (intptr_t)lp;
X+
X }
X
X /*
X@@ -79,51 +76,23 @@
X * Only values returned by "telldir" should be passed to seekdir.
X */
X void
X-_seekdir(dirp, loc)
X- DIR *dirp;
X- long loc;
X+_seekdir(DIR *dirp, long loc)
X {
X- struct ddloc *lp;
X- struct dirent *dp;
X-
X- LIST_FOREACH(lp, &dirp->dd_td->td_locq, loc_lqe) {
X- if (lp->loc_index == loc)
X+ struct dirpos *lp;
X+ for (lp = dirp->dd_internal; lp; lp = lp->dp_next)
X+ if ((intptr_t)lp == loc)
X break;
X- }
X+
X if (lp == NULL)
X return;
X- if (lp->loc_loc == dirp->dd_loc && lp->loc_seek == dirp->dd_seek)
X- goto found;
X- (void) lseek(dirp->dd_fd, (off_t)lp->loc_seek, SEEK_SET);
X- dirp->dd_seek = lp->loc_seek;
X- dirp->dd_loc = 0;
X- while (dirp->dd_loc < lp->loc_loc) {
X- dp = _readdir_unlocked(dirp);
X- if (dp == NULL)
X- break;
X- }
X-found:
X-#ifdef SINGLEUSE
X- LIST_REMOVE(lp, loc_lqe);
X- free((caddr_t)lp);
X-#endif
X-}
X
X-/*
X- * Reclaim memory for telldir cookies which weren't used.
X- */
X-void
X-_reclaim_telldir(dirp)
X- DIR *dirp;
X-{
X- struct ddloc *lp;
X- struct ddloc *templp;
X+ if (lp->dp_loc == dirp->dd_loc && lp->dp_seek == dirp->dd_seek)
X+ return;
X
X- lp = LIST_FIRST(&dirp->dd_td->td_locq);
X- while (lp != NULL) {
X- templp = lp;
X- lp = LIST_NEXT(lp, loc_lqe);
X- free(templp);
X- }
X- LIST_INIT(&dirp->dd_td->td_locq);
X+ dirp->dd_seek = lseek(dirp->dd_fd, lp->dp_seek, SEEK_SET);
X+ dirp->dd_loc = 0;
X+ while (dirp->dd_loc < lp->dp_loc)
X+ if (_readdir_unlocked(dirp) == NULL)
X+ break;
X+
X }
Xdiff -u -r -N lib/libc.old/gen/telldir.h lib/libc/gen/telldir.h
X--- lib/libc.old/gen/telldir.h 2001-01-24 06:59:24.000000000 -0600
X+++ lib/libc/gen/telldir.h 2008-03-12 19:22:10.000000000 -0500
X@@ -38,19 +38,6 @@
X #include <sys/queue.h>
X
X /*
X- * One of these structures is malloced to describe the current directory
X- * position each time telldir is called. It records the current magic
X- * cookie returned by getdirentries and the offset within the buffer
X- * associated with that return value.
X- */
X-struct ddloc {
X- LIST_ENTRY(ddloc) loc_lqe; /* entry in list */
X- long loc_index; /* key associated with structure */
X- long loc_seek; /* magic cookie returned by getdirentries */
X- long loc_loc; /* offset of entry in buffer */
X-};
X-
X-/*
X * One of these structures is malloced for each DIR to record telldir
X * positions.
X */
END-of-./libc.diff
echo x - ./seekdir-twice.c
sed 's/^X//' >./seekdir-twice.c << 'END-of-./seekdir-twice.c'
X#include <stdio.h>
X#include <string.h>
X#include <dirent.h>
X#include <sys/stat.h>
X
Xint main(void) {
X DIR *dp;
X
X mkdir("t", 0755);
X creat("t/a", 0600);
X creat("t/b", 0600);
X creat("t/c", 0600);
X
X if (dp = opendir("t")) {
X char *wasname;
X struct dirent *entry;
X long here;
X
X /* skip two */
X entry = readdir(dp);
X entry = readdir(dp);
X entry = readdir(dp);
X printf("first found: %s\n", entry->d_name);
X
X here = telldir(dp);
X printf("pos returned by telldir: %d\n", here);
X entry = readdir(dp);
X printf("second found: %s\n", entry->d_name);
X
X wasname = strdup(entry->d_name);
X entry = readdir(dp);
X printf("third found: %s\n", entry->d_name);
X
X seekdir(dp, here);
X entry = readdir(dp);
X printf("should be == second: %s\n", entry->d_name);
X
X printf("pos given to seekdir: %d\n", here);
X seekdir(dp, here);
X here = telldir(dp);
X printf("pos returned by telldir: %d\n", here);
X
X entry = readdir(dp);
X printf("should be == second: %s\n", entry->d_name);
X
X seekdir(dp, here);
X entry = readdir(dp);
X printf("should be == second: %s\n", entry->d_name);
X
X closedir(dp);
X }
X return 0;
X}
END-of-./seekdir-twice.c
exit
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200803130101.m2D11IRk024940>