PATCH: realpath() and dead code

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Coverity reported dead code in chrealpath(). A past fix to bail out
early and avoid a null pointer dereference was skipping the code that
tries to repeat realpath/canonicalize_file_name with successive trailing
parts of the path name lopped off until it succeeds. The !real test was
actually tautologous.

I've also removed ELOOP and ENAMETOOLONG from the errors that cause
a bail out: I think it is useful to resolve symlinks in directories
further up the path even if you have cyclic symlinks at the end.

Finally, it seems FreeBSD is one modern system that accepts a
NULL argument to realpath() but lacks the horrendously named
canonicalize_file_name() alternative. So I've added an autoconf test for
that instead. It still checks for canonicalize_file_name but only to
provide a best guess in the case of a cross-compiler.

There doesn't seem to be explicit test cases of :A. I'm not adding any
because I suspect they'd be the sort of thing that fails regularly for
unrelated reasons and because I don't have a clue how symlinks would
work on odd systems like Cygwin and Mac OS.

Oliver

diff --git a/Src/hist.c b/Src/hist.c
index 0831756..7fe843a 100644
--- a/Src/hist.c
+++ b/Src/hist.c
@@ -1702,11 +1702,12 @@ int
 chrealpath(char **junkptr)
 {
     char *str;
-#ifdef HAVE_CANONICALIZE_FILE_NAME
+#ifdef HAVE_REALPATH
+# ifdef REALPATH_ACCEPTS_NULL
     char *lastpos, *nonreal, *real;
-#else
-# ifdef HAVE_REALPATH
-    char *lastpos, *nonreal, real[PATH_MAX];
+# else
+    char *lastpos, *nonreal, pathbuf[PATH_MAX];
+    char *real = pathbuf;
 # endif
 #endif
 
@@ -1717,7 +1718,7 @@ chrealpath(char **junkptr)
     if (!chabspath(junkptr))
 	return 0;
 
-#if !defined(HAVE_REALPATH) && !defined(HAVE_CANONICALIZE_FILE_NAME)
+#ifndef HAVE_REALPATH
     return 1;
 #else
     /*
@@ -1733,29 +1734,21 @@ chrealpath(char **junkptr)
     nonreal = lastpos + 1;
 
     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))
+#ifdef REALPATH_ACCEPTS_NULL
+	   /* realpath() with a NULL second argument uses malloc() to get
+	    * memory so we don't need to worry about overflowing PATH_MAX */
+	   (real = realpath(*junkptr, NULL))
 #else
 	   realpath(*junkptr, real)
 #endif
 	) {
-	if (errno == EINVAL || errno == ELOOP ||
-	    errno == ENAMETOOLONG || errno == ENOMEM)
-	    return 0;
-
-#ifdef HAVE_CANONICALIZE_FILE_NAME
-	if (!real)
+	if (errno == EINVAL || errno == ENOMEM)
 	    return 0;
-#endif
 
 	if (nonreal == *junkptr) {
-	    *real = '\0';
+#ifndef REALPATH_ACCEPTS_NULL
+	    real = NULL;
+#endif
 	    break;
 	}
 
@@ -1771,11 +1764,15 @@ chrealpath(char **junkptr)
 	str++;
     }
 
-    *junkptr = metafy(str = bicat(real, nonreal), -1, META_HEAPDUP);
-    zsfree(str);
-#ifdef HAVE_CANONICALIZE_FILE_NAME
-    free(real);
+    if (real) {
+	*junkptr = metafy(str = bicat(real, nonreal), -1, META_HEAPDUP);
+	zsfree(str);
+#ifdef REALPATH_ACCEPTS_NULL
+	free(real);
 #endif
+    } else {
+	*junkptr = metafy(nonreal, lastpos - nonreal + 1, META_HEAPDUP);
+    }
 #endif
 
     return 1;
diff --git a/configure.ac b/configure.ac
index 56c4cfb..8ea9737 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1302,6 +1302,23 @@ AC_CHECK_FUNCS(strftime strptime mktime timelocal \
 	       cygwin_conv_path)
 AC_FUNC_STRCOLL
 
+AH_TEMPLATE([REALPATH_ACCEPTS_NULL],
+[Define if realpath() accepts NULL as its second argument.])
+AC_CACHE_CHECK([if realpath accepts NULL],
+zsh_cv_func_realpath_accepts_null,
+[AC_RUN_IFELSE([AC_LANG_PROGRAM([
+#include <stdlib.h>
+#include <limits.h>
+],[
+exit(!realpath("/", (char*)0));
+])],
+[zsh_cv_func_realpath_accepts_null=yes],
+[zsh_cv_func_realpath_accepts_null=no],
+[zsh_cv_func_realpath_accepts_null=$ac_cv_func_canonicalize_file_name])])
+if test x$zsh_cv_func_realpath_accepts_null = xyes; then
+  AC_DEFINE(REALPATH_ACCEPTS_NULL)
+fi
+
 if test x$enable_cap = xyes; then
   AC_CHECK_FUNCS(cap_get_proc)
 fi