PATCH: Remove usages of zpathmax(), fix up zopenmax()

[Bart Schaefer <schaefer@xxxxxxxxxxx>]

Following is the reply I got to my posting on comp.std.unix about pathconf.
This confirms my previous impressions; _PC_PATH_MAX, at least, is darn near
useless.

Consequently I've removed the two calls that were made to it and put #if 0
around the whole function definition.  I took the macro out of system.h
and put it (slightly modified) into the function body, after appropriately
shuffling the placement of the #ifdefs.  I expanded the comment to explain
why it's #if'd out, took out the code that attempted to compensate for the
glibc shortcoming but did so wrongly, and added the test for EINVAL.

I also fixed the comments for zopenmax() (we now do use sysconf, so the one
in system.h was obsolete, and the one in compat.c mentioned PATHCONF rather
than SYSCONF), clarified one of them slightly, and reformatted the function
body just a little in the process of poking a space into "if(".

I have not actually attempted to fix the 80-or-so uses of PATH_MAX within
the zsh sources yet.  As of 1992 (the most recent docs I have), POSIX didn't
support symbolic links at all; which explains the confusion Clint and I got
into over readlink(), and may make it rather difficult to come up with good
replacements for a couple of zsh's buffer allocations.

After the patch, files module `mkdir -p' creates intermediate directories
up to the point where one fails, rather than attempting to test path length
in advance; this was already the behavior if an individual file name was
too long (e.g. > 255 bytes) so little is lost.  Also, the parameters module
doesn't reject paths as too long when assigning into the nameddirs special
hash.  Otherwise nothing should have changed.

---------- Forwarded message ----------
Date: Wed, 9 Aug 2000 18:59:31 GMT
From: Paul Eggert <eggert@xxxxxxxxxxx>
Newsgroups: comp.std.unix
Subject: Re: POSIX question: Use of pathconf(_PC_PATH_MAX)

Submitted-by: eggert@xxxxxxxxxxx (Paul Eggert)

schaefer@xxxxxxxxxxx (Bart Schaefer) writes:

>My concern is that there seem to be a lot of possible "legal" behaviors:

Yes.  And in my experience, portable programs should not use symbols
like _PC_PATH_MAX, PATH_MAX, or FILENAME_MAX.  Instead, they should
calculate how long a buffer they need for a particular file name,
and allocate a buffer of that size dynamically.

Programs that preallocate static buffers for arbitrary path names are
asking for trouble, in several ways:

* Some implementations return SIZE_MAX, and it's unwise to
  preallocate, say, a 4 GB buffer to hold a file name.

* pathconf(p,_PC_PATH_MAX) is allowed to return different values
  when called at different times, even for the same value of p.
  This is because the result may depend on the amount of memory
  available.  So it's unwise to rely on the result at all.

* There are a lot of bugs in this area, and life is too short to waste time
  finding implementation bugs for a feature that should be avoided anyway.

* Often, programs that preallocate path name buffers don't have proper checks
  for buffer overrun.

I just ran into this problem with groff 1.16.1.  It preallocates
buffers of size FILENAME_MAX in two places.  In one place, it's easy to
rewrite the code to not allocate a buffer at all.  In the other, it's
easy to rewrite the code to allocate a buffer that is only as large as
needed.  This is a fairly common pattern.


>What's a good test for the pathconf postive-return-value behavior?  Is it
>sufficient to do that test at configure time, or should it be done at run
>time?

Run time, since the test can depend on the filesystem, or on available
memory, or on other things.

---------- End forwarded message ----------

Index: Src/compat.c
===================================================================
@@ -105,7 +105,23 @@
 #endif
 
 
-#ifdef HAVE_PATHCONF
+#if 0
+/* pathconf(_PC_PATH_MAX) is not currently useful to zsh.  The value *
+ * returned varies depending on a number of factors, e.g. the amount *
+ * of memory available to the operating system at a given time; thus *
+ * it can't be used for buffer allocation, or even as an indication  *
+ * of whether an attempt to use or create a given pathname may fail  *
+ * at any future time.                                               *
+ *                                                                   *
+ * The call is also permitted to fail if the argument path is not an *
+ * existing directory, so even to make sense of that one must search *
+ * for a valid directory somewhere in the path and adjust.  Even if  *
+ * it succeeds, the return value is relative to the input directory, *
+ * and therefore potentially relative to the length of the shortest  *
+ * path either to that directory or to our working directory.        *
+ *                                                                   *
+ * Finally, see the note below for glibc; detection of pathconf() is *
+ * not by itself an indication that it works reliably.               */
 
 /* The documentation for pathconf() says something like:             *
  *     The limit is returned, if one exists.  If the system  does    *
@@ -114,34 +130,24 @@
  *     -1  is returned, and errno is set to reflect the nature of    *
  *     the error.                                                    *
  *                                                                   *
- * This is less useful than may be, as one must reset errno to 0 (or *
+ * System calls are not permitted to set errno to 0; but we must (or *
  * some other flag value) in order to determine that the resource is *
  * unlimited.  What use is leaving errno unchanged?  Instead, define *
  * a wrapper that resets errno to 0 and returns 0 for "the system    *
- * does not have a limit," so that -1 always means a real error.     *
- *                                                                   *
- * This is replaced by a macro from system.h if not HAVE_PATHCONF.   *
- *
- * Note that the length of a relative path is compared without first *
- * prepending the current directory, if pathconf() does not return   *
- * an error.  This is for consistency with the macro and with older  *
- * zsh behavior; it may be problematic in the ENOENT/ENOTDIR cases.  */
+ * does not have a limit," so that -1 always means a real error.     */
 
 /**/
 mod_export long
 zpathmax(char *dir)
 {
+#ifdef HAVE_PATHCONF
     long pathmax;
 
     errno = 0;
     if ((pathmax = pathconf(dir, _PC_PATH_MAX)) >= 0) {
-	/* This code is redundant if pathconf works correctly, but   *
-	 * some versions of glibc pathconf return a hardwired value. */
-	if (strlen(dir) < pathmax)
-	    return pathmax;
-	else
-	    errno = ENAMETOOLONG;
-    } else if (errno == ENOENT || errno == ENOTDIR) {
+	/* Some versions of glibc pathconf return a hardwired value! */
+	return pathmax;
+    } else if (errno == EINVAL || errno == ENOENT || errno == ENOTDIR) {
 	/* Work backward to find a directory, until we run out of path. */
 	char *tail = strrchr(dir, '/');
 	while (tail > dir && tail[-1] == '/')
@@ -158,8 +164,9 @@
 		pathmax = pathconf(".", _PC_PATH_MAX);
 	}
 	if (pathmax > 0) {
-	    if (strlen(dir) < pathmax)
-		return pathmax;
+	    long taillen = (tail ? strlen(tail) : (strlen(dir) + 1));
+	    if (taillen < pathmax)
+		return pathmax - taillen;
 	    else
 		errno = ENAMETOOLONG;
 	}
@@ -168,12 +175,20 @@
 	return -1;
     else
 	return 0; /* pathmax should be considered unlimited */
-}
+#else
+    long dirlen = strlen(dir);
+
+    /* The following is wrong if dir is not an absolute path. */
+    return ((long) ((dirlen >= PATH_MAX) ?
+		    ((errno = ENAMETOOLONG), -1) :
+		    ((errno = 0), PATH_MAX - dirlen)));
 #endif
+}
+#endif // 0
 
 #ifdef HAVE_SYSCONF
-/* This is replaced by a macro from system.h if not HAVE_PATHCONF.   *
- * 0 is returned if _SC_OPEN_MAX is unavailable                      *
+/* This is replaced by a macro from system.h if not HAVE_SYSCONF.    *
+ * 0 is returned by sysconf if _SC_OPEN_MAX is unavailable;          *
  * -1 is returned on error                                           *
  *                                                                   *
  * Neither of these should happen, but resort to OPEN_MAX rather     *
@@ -183,10 +198,9 @@
 mod_export long
 zopenmax(void)
 {
-    long openmax;
-    
-    openmax = sysconf(_SC_OPEN_MAX);
-    if(openmax < 1)
+    long openmax = sysconf(_SC_OPEN_MAX);
+
+    if (openmax < 1)
 	return OPEN_MAX;
     else
 	return openmax;
Index: Src/system.h
===================================================================
@@ -200,18 +200,15 @@
 # ifdef MAXPATHLEN
 #  define PATH_MAX MAXPATHLEN
 # else
-   /* so we will just pick something */
-#  define PATH_MAX 1024
+#  ifdef _POSIX_PATH_MAX
+#   define PATH_MAX _POSIX_PATH_MAX
+#  else
+    /* so we will just pick something */
+#   define PATH_MAX 1024
+#  endif
 # endif
 #endif
-#ifndef HAVE_PATHCONF
-# define zpathmax(X) ((long)((strlen(X) >= PATH_MAX) ? \
-			     ((errno = ENAMETOOLONG), -1) : \
-			     ((errno = 0), PATH_MAX)))
-#endif
 
-/* we should be getting this value from sysconf(_SC_OPEN_MAX) */
-/* but this is too much trouble                               */
 #ifndef OPEN_MAX
 # ifdef NOFILE
 #  define OPEN_MAX NOFILE
@@ -221,7 +218,7 @@
 # endif
 #endif
 #ifndef HAVE_SYSCONF
-# define zopenmax() (long) OPEN_MAX
+# define zopenmax() ((long) OPEN_MAX)
 #endif
 
 #ifdef HAVE_FCNTL_H
Index: Src/Modules/files.c
===================================================================
@@ -91,11 +91,6 @@
 
 	while(ptr > *args + (**args == '/') && *--ptr == '/')
 	    *ptr = 0;
-	if(zpathmax(unmeta(*args)) < 0) {
-	    zwarnnam(nam, "%s: %e", *args, errno);
-	    err = 1;
-	    continue;
-	}
 	if(ops['p']) {
 	    char *ptr = *args;
 
Index: Src/Modules/parameter.c
===================================================================
@@ -1397,9 +1397,8 @@
 static void
 setpmnameddir(Param pm, char *value)
 {
-    errno = 0;
-    if (!value || *value != '/' || zpathmax(value) < 0)
-	zwarn((errno ? "%s: %e" : "invalid value: %s"), value, errno);
+    if (!value || *value != '/')
+	zwarn("invalid value: %s", value, 0);
     else
 	adduserdir(pm->nam, value, 0, 1);
     zsfree(value);
@@ -1443,9 +1442,8 @@
 	    v.arr = NULL;
 	    v.pm = (Param) hn;
 
-	    errno = 0;
-	    if (!(val = getstrvalue(&v)) || *val != '/' || zpathmax(val) < 0)
-		zwarn((errno ? "%s: %e" : "invalid value: %s"), val, errno);
+	    if (!(val = getstrvalue(&v)) || *val != '/')
+		zwarn("invalid value: %s", val, 0);
 	    else
 		adduserdir(hn->nam, val, 0, 1);
 	}

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net