Re: cd bugs

[Eric Blake <ebb9@xxxxxxx>]

Eric Blake <ebb9 <at> byu.net> writes:

> $ zsh -c 'emulate -R sh; CDPATH=///; cd eblake; /bin/pwd'
> //eblake
> $ bash -c 'CDPATH=///; cd eblake; /bin/pwd'
> bash: line 0: cd: eblake: No such file or directory
> /tmp
> $

Of the three examples mentioned in this thread, this one is definitely a bug,
but one that only affects platforms where // is special (which, based on the
__CYGWIN__ conditional in the source, appears to be just cygwin).

The other two items mentioned in this thread are POSIX-compliance issues, but as
was pointed out to me off-list, changing them to obey POSIX is a feature request
and not a bug fix.  Unfortunately, while I'd like to help code it up, I'm not
sure how to go about adding a new option to implement POSIX semantics in cd
handling.  Besides adding a new option, one part is simple (in cd_do_chdir, set
hasdot to 1 and fake an implicit '.' entry at the end of cdpath if the POSIX_CD
option is set), the other part is a bit more complex (cdpath has to
differentiate between an explicit "." in CDPATH, which increments doprintdir,
and an implicit '.' due to an explicit empty entry or the implicit '.' added
from the first part of the fix, whereas right now the cdpath variable has
already converted implicit '.' to explicit "." in the array).

Actually, in looking at this patch again, I'm starting to wonder if it might be
better to teach tricat that if the first argument ends in '/' and the second
argument is exactly "/", then it does not need to use the second argument; this
would certainly make it touch all code paths that do file name concatenation,
rather than trying to change down and protect every caller of tricat with
__CYGWIN__ conditionals.  In other words, there are probably also bugs with ~
expansion when $HOME is exactly / or //, as well as other potential gotchas with
// handling that I haven't even investigated here.


From: Eric Blake <ebb9@xxxxxxx>
Date: Tue, 14 Jul 2009 21:01:03 -0600
Subject: [PATCH] Eric Blake: 27149: fix // handling in cd for cygwin

---
 ChangeLog     |    5 +++++
 Src/builtin.c |   14 ++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4255d6f..b9d6dbd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2009-07-14  Eric Blake  <ebb9@xxxxxxx>
+
+	* Eric Blake: 27149: Src/builtin.c: Fix // handling in cd for
+	cygwin.
+
 2009-07-14  Peter Stephenson  <pws@xxxxxxx>

 	* Andy Spencer: 27148: Completion/Linux/Command/_modutils:
diff --git a/Src/builtin.c b/Src/builtin.c
index 62c6e3c..56cc916 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -1029,17 +1029,19 @@ cd_try_chdir(char *pfix, char *dest, int hard)

     /* handle directory prefix */
     if (pfix && *pfix) {
-	if (*pfix == '/')
+	if (*pfix == '/') {
 #ifdef __CYGWIN__
 /* NB: Don't turn "/"+"bin" into "//"+"bin" by mistake!  "//bin" may *
  * not be what user really wants (probably wants "/bin"), but        *
  * "//bin" could be valid too (see fixdir())!  This is primarily for *
- * handling CDPATH correctly.                                        */
-	    buf = tricat(pfix, ( pfix[1] == '\0' ? "" : "/" ), dest);
+ * handling CDPATH correctly.  Likewise for "//"+"bin" not becoming  *
+ * "///bin" (aka "/bin").                                            */
+	    int root = pfix[1] == '\0' || (pfix[1] == '/' && pfix[2] == '\0');
+	    buf = tricat(pfix, ( root ? "" : "/" ), dest);
 #else
 	    buf = tricat(pfix, "/", dest);
 #endif
-	else {
+	} else {
 	    int pfl = strlen(pfix);
 	    dlen = strlen(pwd);

@@ -1200,8 +1202,8 @@ fixdir(char *src)
 	/* compress multiple /es into single */
 	if (*src == '/') {
 #ifdef __CYGWIN__
-	    /* allow leading // under cygwin */
-	    if (src == s0 && src[1] == '/')
+	    /* allow leading // under cygwin, but /// still becomes / */
+	    if (src == s0 && src[1] == '/' && src[2] != '/')
 		*dest++ = *src++;
 #endif
 	    *dest++ = *src++;
-- 
1.6.3.3.334.g916e1