Re: [9fans] p9p: 9 ls /dev

[David Arroyo <droyo@aqwari.net>]

[-- Attachment #1: Type: text/plain, Size: 4492 bytes --]

The problem is twofold;

* The function exits early if it can't read a max-length directory
entry, so that entry is "skipped" in subsequent calls to mygetdents.
* As you said, we also lose any buffered dirents that haven't been
returned from readdir yet.

I think these are both fixable problems, but it may not be worth it.
Ori's suggestion to use Getdirentries64 on OSX might be  better.

David

On Tue, Apr 11, 2017, at 10:33 PM, arisawa wrote:
> I did more test on david code and found a problem.
> 
> -bash$ mk -f mkfile_david
> -bash$ o.test_dirread -a /usr/bin |wc
>     1084    4336   27266
> -bash$ o.test_dirread /usr/bin |wc
>     1084    4336   27266
> -bash$ ls /usr/bin |wc
>     1108    1108    9719
> 
> option -a is for dirreadall.
> 1108 - 1084 entries are lost.
> 
> they are:
> o.test_dirread /usr/bin | awk '{print $1}' | sort >/tmp/a
> ls /usr/bin | sort >/tmp/b
> diff /tmp/a /tmp/b
> 
> -bash$ diff /tmp/a /tmp/b
> 21a22
> > SplitForks
> 240a242,246
> > easy_install-2.6
> > easy_install-2.7
> > efax
> > efix
> > egrep
> 461a468,473
> > kcc
> > kdestroy
> > kextutil
> > keytool
> > kgetcred
> > kill.d
> 675a688,694
> > piconv
> > piconv5.16
> > piconv5.18
> > pidpersec.d
> > pkgbuild
> > pkill
> > pl
> 880a900,904
> > spfquery5.18
> > splain
> > splain5.16
> > splain5.18
> > split
> 
> sorry if I make a mistake, but I suspect readdir_r() has a buffer,
> which can make a problem in using dup().
> 
> Kenji Arisawa.
> 
> > 2017/04/09 13:18、arisawa <karisawa@gmail.com> のメール：
> > 
> > thanks david.
> > 
> > using dup() is very nice idea!
> > your code works with
> >   CFLAGS=-D__DARWIN_64_BIT_INO_T		# manual is wrong
> > and a fix:
> > //	buf = ((char*)buf) + d_reclen(buf);
> > 	buf = (struct dirent *)(((char*)buf) + d_reclen(buf));
> > and adding
> > 	#define NAME_MAX 256
> > in somewhere.
> > 
> > now /dev is readable.
> > 
> > one problem is left.
> > 
> > my test code:
> > 	fd = open(dirname,OREAD);
> > 	if(fd < 0)
> > 		fatal("%s open error",dirname);
> > 	while((n = dirread(fd, &db)) > 0){
> > 		print("#DBG n=%d\n",n);
> > 		for(i = 0; i < n; i++)
> > 			print("%s %s %s \n", db[i].name, db[i].uid, db[i].gid);
> > 	}
> > 	close(fd);
> > shows for dirname=$HOME
> > 	...
> > 	arch root 501 
> > 	bin root 501 
> > 	...
> > but they should be
> > 	arch arisawa staff 
> > 	bin arisawa staff
> > this problem comes from _p9dir() that is used in dirpackage().
> > 
> > Kenji Arisawa
> > 
> > 
> >> 2017/04/08 18:07、David Arroyo <droyo@aqwari.net> のメール：
> >> 
> >> Ignore my previous post, I was tired and forgot about dup(). How about
> >> something like this? (attached)
> >> 
> >> I only tested this on Ubuntu, I don't have an OS X machine. I still went
> >> with readdir_r because the AIX and Solaris man pages for readdir were
> >> vague about its behavior when called from multiple threads (glibc, musl,
> >> FreeBSD look pretty safe).
> >> 
> >> Cheers,
> >> David
> >> 
> >> On Sat, Apr 8, 2017, at 03:27 AM, David Arroyo wrote:
> >>> This should be doable with some combination of fdopendir(3) and
> >>> readdir(3). I'm not sure how to avoid leaking memory through the
> >>> returned DIR pointer and any memory allocated with by readdir(3).
> >>> This is usually free'd by closedir(3), which we can't use without
> >>> closing the underlying file.
> >>> 
> >>> It should be OK to use free() on the return value of fdopendir, and
> >>> stick to the uglier readdir_r(3) interface. I can definitely see why
> >>> Russ went with  the simpler  system-specific interfaces on this.
> >>> 
> >>> David
> >>> 
> >>> On Sat, Apr 8, 2017, at 02:46 AM, Ori Bernstein wrote:
> >>>> On Sat, 8 Apr 2017 15:21:47 +0900, arisawa <karisawa@gmail.com> wrote:
> >>>> 
> >>>>> but how to?
> >>>>> 
> >>>>> unix doesn’t have something like fdreaddir(int fd).
> >>>>> my guess: russ unwillingly used a low level function such as
> >>>>> int getdirentries(int fd, char *buf, int nbytes, long *basep).
> >>>>> 
> >>>>> readdirall() might be OK in regular usage.
> >>>> 
> >>>> I don't use OSX regularly, although I do maintain the syscall
> >>>> layer for Myrddin on it.
> >>>> 
> >>>> Getdirentries64 exists, and rudimentary testing doesn't show
> >>>> any difficulties with using it.
> >>>> 
> >>>> -- 
> >>>>   Ori Bernstein
> >>>> 
> >>> 
> >> <posix-dirread.patch>
> > 
> 
> 

[-- Attachment #2: posix-dirread2.patch --]
[-- Type: text/plain, Size: 1880 bytes --]

diff --git a/src/lib9/dirread.c b/src/lib9/dirread.c
index 40fbe3c..3b70938 100644
--- a/src/lib9/dirread.c
+++ b/src/lib9/dirread.c
@@ -6,46 +6,6 @@
 
 extern int _p9dir(struct stat*, struct stat*, char*, Dir*, char**, char*);
 
-#if defined(__linux__)
-static int
-mygetdents(int fd, struct dirent *buf, int n)
-{
-	off_t off;
-	int nn;
-
-	/* This doesn't match the man page, but it works in Debian with a 2.2 kernel */
-	off = p9seek(fd, 0, 1);
-	nn = getdirentries(fd, (void*)buf, n, &off);
-	return nn;
-}
-#elif defined(__APPLE__) 
-static int
-mygetdents(int fd, struct dirent *buf, int n)
-{
-	long off;
-	return getdirentries(fd, (void*)buf, n, &off);
-}
-#elif defined(__FreeBSD__) || defined(__DragonFly__)
-static int
-mygetdents(int fd, struct dirent *buf, int n)
-{
-	off_t off;
-	return getdirentries(fd, (void*)buf, n, &off);
-}
-#elif defined(__sun__) || defined(__NetBSD__) || defined(__OpenBSD__)
-static int
-mygetdents(int fd, struct dirent *buf, int n)
-{
-	return getdents(fd, (void*)buf, n);
-}
-#elif defined(__AIX__)
-static int
-mygetdents(int fd, struct dirent *buf, int n)
-{
-	return getdirent(fd, (void*)buf, n);
-}
-#endif
-
 #if defined(__DragonFly__)
 static inline int d_reclen(struct dirent *de) { return _DIRENT_DIRSIZ(de); }
 #else
@@ -53,6 +13,31 @@ static inline int d_reclen(struct dirent *de) { return de->d_reclen; }
 #endif
 
 static int
+mygetdents(int fd, struct dirent *buf, int n)
+{
+	int tmpfd;
+	char *p;
+	DIR *dirp;
+	struct dirent *entry;
+	
+	if ((tmpfd = dup(fd)) == -1)
+		return -1;
+	if ((dirp = fdopendir(tmpfd)) == nil)
+		return -1;
+	
+	p = (char*)buf;
+	while((entry = readdir(dirp)) != nil) {
+		if (d_reclen(entry) > n)
+			break;
+
+		memcpy(p, entry, d_reclen(entry));
+		p += d_reclen(entry);
+		n -= d_reclen(entry);
+	}
+	return (int)(p - (char*)buf);
+}
+
+static int
 countde(char *p, int n)
 {
 	char *e;