Re: zsh 5.0.5-dev-2

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Aug 14,  8:54pm, Peter Stephenson wrote:
}
} It's entirely unclear to me from looking at xsymlinks() when an empty
} xbuf would actually constitute a failure.

This occurs in the (t0 == -1) case when the reason that readlink() has
failed is because the path so far concatenated with the next segment of
the split path has produced a string longer than PATH_MAX.  In that case
(xbuflen + pplen > sizeof(xbuf)) we completely truncate xbuf and give up.

} If it can't expand something
} as a symbolic link it simply treats it literally;

Yes, but for a /path/to/somereallylongname, appending somereallylongname
to the expansion of xbuf so far, might overflow xbuf.

} If we don't have an answer now I'd be inclined to comment out the
} warning.

Maybe just make xsymlinks() have a ternary return value instead of a
boolean one, and return an actual error value in the case where we've
truncated on overflow instead of as part of normal expansion to root?

I don't know if all of these changes are necessary.  It has always
bothered me that the return value of the recursive calls to xsymlinks()
are ignored, but maybe this isn't the right way to handle them.

Incidentally, anybody recall what the obviously obsolete comment above
the xsymlinks() definition in Src/utils.c is about?

diff --git a/Src/utils.c b/Src/utils.c
index 998e46a..076a33c 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -716,7 +716,6 @@ slashsplit(char *s)
 }
 
 /* expands symlinks and .. or . expressions */
-/* if flag = 0, only expand .. and . expressions */
 
 /**/
 static int
@@ -753,6 +752,7 @@ xsymlinks(char *s)
 		strcat(xbuf, *pp);
 	    } else {
 		*xbuf = 0;
+		ret = -1;
 		break;
 	    }
 	} else {
@@ -760,9 +760,11 @@ xsymlinks(char *s)
 	    metafy(xbuf3, t0, META_NOALLOC);
 	    if (*xbuf3 == '/') {
 		strcpy(xbuf, "");
-		xsymlinks(xbuf3 + 1);
+		if (xsymlinks(xbuf3 + 1) < 0)
+		    ret = -1;
 	    } else
-		xsymlinks(xbuf3);
+		if (xsymlinks(xbuf3) < 0)
+		    ret = -1;
 	}
     }
     freearray(opp);
@@ -781,11 +783,10 @@ xsymlink(char *s)
     if (*s != '/')
 	return NULL;
     *xbuf = '\0';
-    xsymlinks(s + 1);
-    if (!*xbuf) {
+    if (xsymlinks(s + 1) < 0)
 	zwarn("path expansion failed, using root directory");
+    if (!*xbuf)
 	return ztrdup("/");
-    }
     return ztrdup(xbuf);
 }
 
@@ -795,7 +796,7 @@ print_if_link(char *s)
 {
     if (*s == '/') {
 	*xbuf = '\0';
-	if (xsymlinks(s + 1))
+	if (xsymlinks(s + 1) > 0)
 	    printf(" -> "), zputs(*xbuf ? xbuf : "/", stdout);
     }
 }

-- 
Barton E. Schaefer