Re: cd -s symlink hangs (sometimes?)

[Peter Stephenson <p.w.stephenson@ntlworld.com>]

On Fri, 20 Mar 2009 22:12:30 +0100
Mikael Magnusson <mikachu@gmail.com> wrote:
> zsh -f
> % mkdir a
> % ln -s a b
> % cd -s b
> cd: not a directory: b
> % cd -s b
> # kill -9 in another terminal
> zsh: killed     zsh -f

The following is reproducible (note I didn't actually create the
directory a):

% zsh -f
% cd ~
% pwd
/home/pws
% rm -f a b
% ln -s a b
% cd -s b
cd: not a directory: b
% /bin/pwd
/

The -s does seem to be the crucial factor---which figures, since cd is
very heavily used so this would otherwise have turned up long ago.  This
is related to the lchdir() / zgetdir() code I was looking at the other
day and thinking was rather hairy (but the bug goes back to the 4.2 line
and presumably much further).

I can't be sure the fix below doesn't introduce another subtle problem,
though I think the test "d->dirfd < 0" should protect us from the most
likely side effects.  The underlying problem here is that the code for
HAVE_FCHDIR appears to have been kludged in without fixing the code
structure to do it properly.  I really don't like that complicated
expression I've moved down to fix the problem: in case we couldn't open
the current directory, investigate what directory it is, check it's not
absolute and if so open the parent directory instead... what is that
palaver supposed to do?

Still, I suspect it's better with rather than without the change.

Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.215
diff -u -r1.215 utils.c
--- Src/utils.c	20 Mar 2009 20:52:13 -0000	1.215
+++ Src/utils.c	20 Mar 2009 22:41:43 -0000
@@ -5388,11 +5388,7 @@
     if (*path == '/') {
 #endif
 	level = -1;
-#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);
-#else
+#ifndef HAVE_FCHDIR
 	if (!d->dirname)
 	    zgetdir(d);
 #endif
@@ -5404,6 +5400,11 @@
 	    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)
Index: Test/B01cd.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/B01cd.ztst,v
retrieving revision 1.3
diff -u -r1.3 B01cd.ztst
--- Test/B01cd.ztst	5 Aug 2002 13:10:02 -0000	1.3
+++ Test/B01cd.ztst	20 Mar 2009 22:41:43 -0000
@@ -109,6 +109,14 @@
 >$mydir/cdtst.tmp/real
 >$mydir/cdtst.tmp/real
 
+ ln -s nonexistent link_to_nonexistent
+ pwd1=$(pwd -P)
+ cd -s link_to_nonexistent
+ pwd2=$(pwd -P)
+ [[ $pwd1 = $pwd2 ]] || print "Ooops, changed to directory '$pwd2'"
+0:
+?(eval):cd:3: not a directory: link_to_nonexistent
+
 %clean
 # This optional section cleans up after the test, if necessary,
 # e.g. killing processes etc.  This is in addition to the removal of *.tmp


-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/