From: Rob Pike <r@golang.org>
Date: Tue, 10 Feb 2009 19:55:48 +0000 (-0800)
Subject: Make Readdirnames work properly on Linux.
X-Git-Tag: weekly.2009-11-06~2194
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=00b3d48f13957d60e1d5029ca35bb8069c069e02;p=gostls13.git

Make Readdirnames work properly on Linux.
Refactor so Readdir is portable code.

R=rsc
DELTA=192  (50 added, 130 deleted, 12 changed)
OCL=24770
CL=24772
---

diff --git a/src/lib/os/dir_amd64_darwin.go b/src/lib/os/dir_amd64_darwin.go
index e66f540c85..04d3a90c6f 100644
--- a/src/lib/os/dir_amd64_darwin.go
+++ b/src/lib/os/dir_amd64_darwin.go
@@ -64,63 +64,3 @@ func Readdirnames(fd *FD, count int) (names []string, err *os.Error) {
 	}
 	return names, nil
 }
-
-// TODO(r): see comment in dir_amd64_linux.go
-
-// Negative count means read until EOF.
-func Readdir(fd *FD, count int) (dirs []Dir, err *os.Error) {
-	dirname := fd.name;
-	if dirname == "" {
-		dirname = ".";
-	}
-	dirname += "/";
-	// Getdirentries needs the file offset - it's too hard for the kernel to remember
-	// a number it already has written down.
-	base, err1 := syscall.Seek(fd.fd, 0, 1);
-	if err1 != 0 {
-		return nil, os.ErrnoToError(err1)
-	}
-	// The buffer must be at least a block long.
-	// TODO(r): use fstatfs to find fs block size.
-	var buf = make([]byte, blockSize);
-	dirs = make([]Dir, 0, 100);	// TODO: could be smarter about size
-	for {
-		if count == 0 {
-			break
-		}
-		ret, err2 := syscall.Getdirentries(fd.fd, &buf[0], int64(len(buf)), &base);
-		if ret < 0 || err2 != 0 {
-			return dirs, os.ErrnoToError(err2)
-		}
-		if ret == 0 {
-			break
-		}
-		for w, i := uintptr(0),uintptr(0); i < uintptr(ret); i += w {
-			if count == 0 {
-				break
-			}
-			dirent := unsafe.Pointer((uintptr(unsafe.Pointer(&buf[0])) + i)).(*syscall.Dirent);
-			w = uintptr(dirent.Reclen);
-			if dirent.Ino == 0 {
-				continue
-			}
-			count--;
-			if len(dirs) == cap(dirs) {
-				ndirs := make([]Dir, len(dirs), 2*len(dirs));
-				for i := 0; i < len(dirs); i++ {
-					ndirs[i] = dirs[i]
-				}
-				dirs = ndirs;
-			}
-			dirs = dirs[0:len(dirs)+1];
-			filename := string(dirent.Name[0:dirent.Namlen]);
-			dirp, err := Lstat(dirname + filename);
-			if dirp == nil || err != nil {
-				dirs[len(dirs)-1].Name = filename;	// rest will be zeroed out
-			} else {
-				dirs[len(dirs)-1] = *dirp;
-			}
-		}
-	}
-	return dirs, nil;
-}
diff --git a/src/lib/os/dir_amd64_linux.go b/src/lib/os/dir_amd64_linux.go
index bd81146aea..a5b8a7ba2e 100644
--- a/src/lib/os/dir_amd64_linux.go
+++ b/src/lib/os/dir_amd64_linux.go
@@ -10,6 +10,10 @@ import (
 	"unsafe";
 )
 
+const (
+	blockSize = 4096	// TODO(r): use statfs
+)
+
 func clen(n []byte) int {
 	for i := 0; i < len(n); i++ {
 		if n[i] == 0 {
@@ -21,28 +25,38 @@ func clen(n []byte) int {
 
 // Negative count means read until EOF.
 func Readdirnames(fd *FD, count int) (names []string, err *os.Error) {
-	// The buffer should be at least a block long.
-	// TODO(r): use fstatfs to find fs block size.
-	var buf = make([]syscall.Dirent, 8192/unsafe.Sizeof(*new(syscall.Dirent)));
-	names = make([]string, 0, 100);	// TODO: could be smarter about size
-	for {
-		if count == 0 {
-			break
-		}
-		ret, err2 := syscall.Getdents(fd.fd, &buf[0], int64(len(buf) * unsafe.Sizeof(buf[0])));
-		if ret < 0 || err2 != 0 {
-			return names, os.ErrnoToError(err2)
-		}
-		if ret == 0 {
-			break
-		}
-		for w, i := uintptr(0),uintptr(0); i < uintptr(ret); i += w {
-			if count == 0 {
-				break
+	// If this fd has no dirinfo, create one.
+	if fd.dirinfo == nil {
+		fd.dirinfo = new(DirInfo);
+		// The buffer must be at least a block long.
+		// TODO(r): use fstatfs to find fs block size.
+		fd.dirinfo.buf = make([]byte, blockSize);
+	}
+	d := fd.dirinfo;
+	size := count;
+	if size < 0 {
+		size = 100
+	}
+	names = make([]string, 0, size);	// Empty with room to grow.
+	for count != 0 {
+		// Refill the buffer if necessary
+		if d.bufp == d.nbuf {
+			var errno int64;
+			dbuf := unsafe.Pointer(&d.buf[0]).(*syscall.Dirent);
+			d.nbuf, errno = syscall.Getdents(fd.fd, dbuf, int64(len(d.buf)));
+			if d.nbuf < 0 {
+				return names, os.ErrnoToError(errno)
 			}
-			dirent := unsafe.Pointer((uintptr(unsafe.Pointer(&buf[0])) + i)).(*syscall.Dirent);
-			w = uintptr(dirent.Reclen);
-			if dirent.Ino == 0 {
+			if d.nbuf == 0 {
+				break	// EOF
+			}
+			d.bufp = 0;
+		}
+		// Drain the buffer
+		for count != 0 && d.bufp < d.nbuf {
+			dirent := unsafe.Pointer(&d.buf[d.bufp]).(*syscall.Dirent);
+			d.bufp += int64(dirent.Reclen);
+			if dirent.Ino == 0 {	// File absent in directory.
 				continue
 			}
 			count--;
@@ -59,64 +73,3 @@ func Readdirnames(fd *FD, count int) (names []string, err *os.Error) {
 	}
 	return names, nil;
 }
-
-// TODO(r): Readdir duplicates a lot of Readdirnames. The other way would
-// be to have Readdir (which could then be portable) call Readdirnames and
-// then do the Stats.  The existing design was chosen to avoid allocating a
-// throwaway names array, but the issue should be revisited once we have
-// a better handle on what that overhead is with a strong garbage collector.
-// Also, it's possible given the nature of the Unix kernel that interleaving
-// reads of the directory with stats (as done here) would work better than
-// one big read of the directory followed by a long run of Stat calls.
-
-// Negative count means read until EOF.
-func Readdir(fd *FD, count int) (dirs []Dir, err *os.Error) {
-	dirname := fd.name;
-	if dirname == "" {
-		dirname = ".";
-	}
-	dirname += "/";
-	// The buffer must be at least a block long.
-	// TODO(r): use fstatfs to find fs block size.
-	var buf = make([]syscall.Dirent, 8192/unsafe.Sizeof(*new(syscall.Dirent)));
-	dirs = make([]Dir, 0, 100);	// TODO: could be smarter about size
-	for {
-		if count == 0 {
-			break
-		}
-		ret, err2 := syscall.Getdents(fd.fd, &buf[0], int64(len(buf) * unsafe.Sizeof(buf[0])));
-		if ret < 0 || err2 != 0 {
-			return dirs, os.ErrnoToError(err2)
-		}
-		if ret == 0 {
-			break
-		}
-		for w, i := uintptr(0),uintptr(0); i < uintptr(ret); i += w {
-			if count == 0 {
-				break
-			}
-			dirent := unsafe.Pointer((uintptr(unsafe.Pointer(&buf[0])) + i)).(*syscall.Dirent);
-			w = uintptr(dirent.Reclen);
-			if dirent.Ino == 0 {
-				continue
-			}
-			count--;
-			if len(dirs) == cap(dirs) {
-				ndirs := make([]Dir, len(dirs), 2*len(dirs));
-				for i := 0; i < len(dirs); i++ {
-					ndirs[i] = dirs[i]
-				}
-				dirs = ndirs;
-			}
-			dirs = dirs[0:len(dirs)+1];
-			filename := string(dirent.Name[0:clen(dirent.Name)]);
-			dirp, err := Stat(dirname + filename);
-			if dirp ==  nil || err != nil {
-				dirs[len(dirs)-1].Name = filename;	// rest will be zeroed out
-			} else {
-				dirs[len(dirs)-1] = *dirp;
-			}
-		}
-	}
-	return dirs, nil;
-}
diff --git a/src/lib/os/os_file.go b/src/lib/os/os_file.go
index cd924bd205..2a4bc6723d 100644
--- a/src/lib/os/os_file.go
+++ b/src/lib/os/os_file.go
@@ -160,3 +160,30 @@ func Lstat(name string) (dir *Dir, err *Error) {
 	}
 	return dirFromStat(name, new(Dir), stat), nil
 }
+
+// Non-portable function defined in operating-system-dependent file.
+func Readdirnames(fd *FD, count int) (names []string, err *os.Error)
+
+// Negative count means read until EOF.
+func Readdir(fd *FD, count int) (dirs []Dir, err *os.Error) {
+	dirname := fd.name;
+	if dirname == "" {
+		dirname = ".";
+	}
+	dirname += "/";
+	names, err1 := Readdirnames(fd, count);
+	if err1 != nil {
+		return nil, err1
+	}
+	dirs = make([]Dir, len(names));
+	for i, filename := range names {
+		dirp, err := Stat(dirname + filename);
+		if dirp ==  nil || err != nil {
+			dirs[i].Name = filename	// rest is already zeroed out
+		} else {
+			dirs[i] = *dirp
+		}
+	}
+	return
+}
+