Re: Modifiers, command position, and so forth (Re: Bug#519535: history expansion: modifier completion missing)

[Peter Stephenson <pws@csr.com>]

On Wed, 18 Mar 2009 19:36:17 +0000
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> I see it uses realpath() for normalising symbolic links.  I'm not sure
> how standard that is (or rather used to be), and the Linux manual warns
> about possible size problems.  We should probably use an improved
> version of zgetdir(), which could also benefit from having PATH_MAX
> removed.  However, zgetdir() is completely undocumented, so that's for
> later.

zgetdir() appears to be quite hairy enough already.  (I wonder if we still
need those holdintr()s---wouldn't queue_interrupts() be more appropriate?)
I've added some comments, fixed a misplaced #ifdef, then ignored it.

This tests for realpath() and the GNU extension that allows the function to
malloc() the path.  I've taken the easy way out and used the alternative
interface to the latter, but if there are other systems that allocate the
path when NULL is passed as the second argument to realpath() but don't
have canonicalize_file_name() it would probably be worth doing a configure
test for working realpath(path, NULL) instead.

There was also another stray variable declaration in the middle of the
function which an older compiler wouldn't like.

Index: configure.ac
===================================================================
RCS file: /cvsroot/zsh/zsh/configure.ac,v
retrieving revision 1.121
diff -u -r1.121 configure.ac
--- configure.ac	16 Mar 2009 05:20:36 -0000	1.121
+++ configure.ac	19 Mar 2009 14:27:47 -0000
@@ -1157,7 +1157,8 @@
 	       grantpt unlockpt ptsname \
 	       htons ntohs \
 	       regcomp regexec regerror regfree \
-	       gdbm_open getxattr)
+	       gdbm_open getxattr \
+	       realpath canonicalize_file_name)
 AC_FUNC_STRCOLL
 
 if test x$enable_cap = xyes; then
Index: Doc/Zsh/expn.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/expn.yo,v
retrieving revision 1.103
diff -u -r1.103 expn.yo
--- Doc/Zsh/expn.yo	15 Mar 2009 01:17:06 -0000	1.103
+++ Doc/Zsh/expn.yo	19 Mar 2009 14:27:47 -0000
@@ -223,7 +223,8 @@
 item(tt(A))(
 As `tt(a)', but also resolve use of symbolic links where possible.
 Note that resolution of `tt(..)' occurs em(before) resolution of symbolic
-links.
+links.  This call is equivalent to tt(a) unless your system has the
+tt(realpath) system call (modern systems do).
 )
 item(tt(e))(
 Remove all but the extension.
Index: Src/compat.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/compat.c,v
retrieving revision 1.17
diff -u -r1.17 compat.c
--- Src/compat.c	22 Apr 2008 15:08:12 -0000	1.17
+++ Src/compat.c	19 Mar 2009 14:27:48 -0000
@@ -227,6 +227,26 @@
 }
 #endif
 
+/*
+ * Rationalise the current directory, returning the string.
+ *
+ * If "d" is not NULL, it is used to store information about the
+ * directory.  The returned name is also present in d->dirname and is in
+ * permanently allocated memory.  The handling of this case depends on
+ * whether the fchdir() system call is available; if it is, it is assumed
+ * the caller is able to restore the current directory.  On successfully
+ * identifying the directory the function returns immediately rather
+ * than ascending the hierarchy.
+ *
+ * If "d" is NULL, no assumption about the caller's behaviour is
+ * made.  The returned string is in heap memory.  This case is
+ * always handled by changing directory up the hierarchy.
+ *
+ * On Cygwin or other systems where USE_GETCWD is defined (at the
+ * time of writing only QNX), we skip all the above and use the
+ * getcwd() system call.
+ */
+
 /**/
 mod_export char *
 zgetdir(struct dirsav *d)
@@ -257,25 +277,30 @@
 	return buf;
     }
 
+    /* Record the initial inode and device */
     pino = sbuf.st_ino;
     pdev = sbuf.st_dev;
     if (d)
 	d->ino = pino, d->dev = pdev;
+#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 #ifdef HAVE_FCHDIR
     else
 #endif
-#if !defined(__CYGWIN__) && !defined(USE_GETCWD)
 	holdintr();
 
     for (;;) {
+	/* Examine the parent of the current directory. */
 	if (stat("..", &sbuf) < 0)
 	    break;
 
+	/* Inode and device of curtent directory */
 	ino = pino;
 	dev = pdev;
+	/* Inode and device of current directory's parent */
 	pino = sbuf.st_ino;
 	pdev = sbuf.st_dev;
 
+	/* If they're the same, we've reached the root directory. */
 	if (ino == pino && dev == pdev) {
 	    if (!buf[pos])
 		buf[--pos] = '/';
@@ -291,6 +316,7 @@
 	    return buf + pos;
 	}
 
+	/* Search the parent for the current directory. */
 	if (!(dir = opendir("..")))
 	    break;
 
@@ -303,6 +329,7 @@
 		continue;
 #ifdef HAVE_STRUCT_DIRENT_D_STAT
 	    if(de->d_stat.st_dev == dev && de->d_stat.st_ino == ino) {
+		/* Found the directory we're currently in */
 		strncpy(nbuf + 3, fn, PATH_MAX);
 		break;
 	    }
@@ -311,6 +338,7 @@
 	    if (dev != pdev || (ino_t) de->d_ino == ino)
 # endif /* HAVE_STRUCT_DIRENT_D_INO */
 	    {
+		/* Maybe found directory, need to check device & inode */
 		strncpy(nbuf + 3, fn, PATH_MAX);
 		lstat(nbuf, &sbuf);
 		if (sbuf.st_dev == dev && sbuf.st_ino == ino)
@@ -320,7 +348,7 @@
 	}
 	closedir(dir);
 	if (!de)
-	    break;
+	    break;		/* Not found */
 	len = strlen(nbuf + 2);
 	pos -= len;
 	while (pos <= 1) {
Index: Src/hist.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/hist.c,v
retrieving revision 1.90
diff -u -r1.90 hist.c
--- Src/hist.c	15 Mar 2009 01:17:06 -0000	1.90
+++ Src/hist.c	19 Mar 2009 14:27:48 -0000
@@ -1522,7 +1522,7 @@
 	current += 3;
     }
 #endif
-	
+
     for (;;) {
 	if (*current == '/') {
 #ifdef __CYGWIN__
@@ -1577,7 +1577,14 @@
 int
 chrealpath(char **junkptr)
 {
+    char *str;
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+    char *lastpos, *nonreal, *real;
+#else
+# ifdef HAVE_REAL_PATH
     char *lastpos, *nonreal, real[PATH_MAX];
+# endif
+#endif
 
     if (!**junkptr)
 	return 1;
@@ -1586,6 +1593,9 @@
     if (!chabspath(junkptr))
 	return 0;
 
+#if !defined(HAVE_REALPATH) && !defined(HAVE_CANONICALIZE_FILE_NAME)
+    return 1;
+#else
     /*
      * Notice that this means you cannot pass relative paths into this
      * function!
@@ -1596,7 +1606,19 @@
     lastpos = strend(*junkptr);
     nonreal = lastpos + 1;
 
-    while (!realpath(*junkptr, real)) {
+    while (!
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+	   /*
+	    * This is a GNU extension to realpath(); it's the
+	    * same as calling realpath() with a NULL second argument
+	    * which uses malloc() to get memory.  The alternative
+	    * interface is easier to test for, however.
+	    */
+	   (real = canonicalize_file_name(*junkptr))
+#else
+	   realpath(*junkptr, real)
+#endif
+	) {
 	if (errno == EINVAL || errno == ELOOP ||
 	    errno == ENAMETOOLONG || errno == ENOMEM)
 	    return 0;
@@ -1611,7 +1633,7 @@
 	*nonreal = '\0';
     }
 
-    char *str = nonreal;
+    str = nonreal;
     while (str <= lastpos) {
 	if (*str == '\0')
 	    *str = '/';
@@ -1619,6 +1641,10 @@
     }
 
     *junkptr = bicat(real, nonreal);
+#ifdef HAVE_CANONICALIZE_FILE_NAME
+    free(real);
+#endif
+#endif
 
     return 1;
 }


-- 
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