Re: cd -s symlink hangs (sometimes?)

[Peter Stephenson <pws@csr.com>]

On Mon, 23 Mar 2009 00:05:10 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> I just noticed (ls -l /proc/$$/fd) that with this last commit, zsh now keeps
> all my old directories open, even when I don't use -s or anything special.

Lucky you noticed.

This is more overload-things-in-a-complicated-fashion-without-documentation
fallout.

The following should be safer, although the bug you noticed was, I think,
just because we didn't need to open the directory at all if allowed to
follow links because zchdir doesn't use restoredir().  So close_dir
may be redundant, but it's hard to tell.

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.216
diff -u -r1.216 utils.c
--- Src/utils.c	20 Mar 2009 22:52:56 -0000	1.216
+++ Src/utils.c	23 Mar 2009 10:46:11 -0000
@@ -5374,6 +5374,9 @@
     int err;
     struct stat st2;
 #endif
+#ifdef HAVE_FCHDIR
+    int close_dir = 0;
+#endif
 
     if (!d) {
 	ds.ino = ds.dev = 0;
@@ -5400,11 +5403,6 @@
 	    d->ino = st1.st_ino;
 	}
     }
-#ifdef HAVE_FCHDIR
-    if (d->dirfd < 0 && (d->dirfd = open(".", O_RDONLY | O_NOCTTY)) < 0 &&
-	zgetdir(d) && *d->dirname != '/')
-	d->dirfd = open("..", O_RDONLY | O_NOCTTY);
-#endif
 
 #ifdef HAVE_LSTAT
     if (!hard)
@@ -5420,7 +5418,16 @@
 	}
 	return zchdir((char *) path);
     }
+
 #ifdef HAVE_LSTAT
+#ifdef HAVE_FCHDIR
+    if (d->dirfd < 0) {
+	close_dir = 1;
+        if ((d->dirfd = open(".", O_RDONLY | O_NOCTTY) < 0) &&
+	    zgetdir(d) && *d->dirname != '/')
+	    d->dirfd = open("..", O_RDONLY | O_NOCTTY);
+    }
+#endif
     if (*path == '/')
 	if (chdir("/") < 0)
 	    zwarn("failed to chdir(/): %e", errno);
@@ -5428,12 +5435,16 @@
 	while(*path == '/')
 	    path++;
 	if(!*path) {
-	    if (d == &ds) {
+	    if (d == &ds)
 		zsfree(ds.dirname);
-		if (ds.dirfd >=0)
-		    close(ds.dirfd);
-	    } else
+	    else
 		d->level = level;
+#ifdef HAVE_FCHDIR
+	    if (d->dirfd >=0 && close_dir) {
+		close(d->dirfd);
+		d->dirfd = -1;
+	    }
+#endif
 	    return 0;
 	}
 	for(pptr = path; *++pptr && *pptr != '/'; ) ;
@@ -5468,19 +5479,25 @@
 	}
     }
     if (restoredir(d)) {
-	if (d == &ds) {
+	if (d == &ds)
 	    zsfree(ds.dirname);
-	    if (ds.dirfd >=0)
-		close(ds.dirfd);
+#ifdef HAVE_FCHDIR
+	if (d->dirfd >=0 && close_dir) {
+	    close(d->dirfd);
+	    d->dirfd = -1;
 	}
+#endif
 	errno = err;
 	return -2;
     }
-    if (d == &ds) {
+    if (d == &ds)
 	zsfree(ds.dirname);
-	if (ds.dirfd >=0)
-	    close(ds.dirfd);
+#ifdef HAVE_FCHDIR
+    if (d->dirfd >=0 && close_dir) {
+	close(d->dirfd);
+	d->dirfd = -1;
     }
+#endif
     errno = err;
     return -1;
 #endif /* HAVE_LSTAT */


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070