Dump hacky overoptimisation in addir -- it doesn't actually get the case

of symlinks to directories right. Patch from Eric Gillespie.

diff --git a/ChangeLog b/ChangeLog
index ce6939b..5f02b17 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,12 @@
 2008-08-04  Eric Gillespie  <epg@pretzelnet.org>
 
-       * uip/folder.c: Simplify dodir/addir/addfold.
+       * uip/folder.c: Simplify dodir/addir/addfold.  Dump hacky
+       over-optimization in addir that tried to avoid readdir after all
+       child directories had been read; this was also trying to support
+       symlinks to directories, but would have been failing (because
+       nlink may have gone to 0 with symlinks to directories remaining)
+       had the lstat usage been correct (lstat doesn't fail for normal
+       directories; should have used S_ISLNK).
 
 2008-08-03  Peter Maydell  <pmaydell@chiark.greenend.org.uk>
 

diff --git a/uip/folder.c b/uip/folder.c
index a0821c2..d4d9868 100644 (file)
--- a/uip/folder.c
+++ b/uip/folder.c
@@ -665,16 +665,11 @@ sfold (struct msgs *mp, char *msg)
 static void
 addir (char *name)
 {
-    int nlink;
     char *prefix, *child;
     struct stat st;
     struct dirent *dp;
     DIR * dd;
 
-   /* short-cut to see if directory has any sub-directories */
-    if (stat (name, &st) != -1 && st.st_nlink == 2)
-        return;
- 
     if (!(dd = opendir (name))) {
        admonish (name, "unable to read directory ");
        return;
@@ -686,25 +681,12 @@ addir (char *name)
        prefix = concat (name, "/", (void *)NULL);
     }
 
-    /*
-     * Keep track of the number of directories we've seen
-     * so we can quit stat'ing early, if we've seen them all.
-     */
-    nlink = st.st_nlink;
-
-    while (nlink && (dp = readdir (dd))) {
+    while ((dp = readdir (dd))) {
        if (!strcmp (dp->d_name, ".") || !strcmp (dp->d_name, "..")) {
-           nlink--;
            continue;
        }
        child = concat (prefix, dp->d_name, (void *)NULL);
        if (stat (child, &st) != -1 && S_ISDIR(st.st_mode)) {
-           /*
-            * Check if this was really a symbolic link pointing at
-            * a directory.  If not, then decrement link count.
-            */
-           if (lstat (child, &st) == -1)
-               nlink--;
            /* addfold saves child in the list, don't free it */
            addfold (child);
        } else {