Re: [BUG] Directory glob picks up running or already-run scripts on OS X

[Peter Stephenson <p.stephenson@xxxxxxxxxxx>]

On Sun, 10 Jul 2016 21:58:21 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> I'm not sure whether this was introduced when adding the (Y) qualifier,
> or if it was already lurking.  With the following DPUTS() added and zsh
> compiled with debugging, Test/D02glob.ztst will fail (on any platform).
> 
> diff --git a/Src/glob.c b/Src/glob.c
> index 2051016..5260b63 100644
> --- a/Src/glob.c
> +++ b/Src/glob.c
> @@ -318,6 +318,8 @@ insert(char *s, int checked)
>      char *news = s;
>      int statted = 0;
>  
> +    DPUTS(!s || !*s, "BUG: adding empty string as glob match");
> +
>      queue_signals();
>      inserts = NULL;
>  
> 
> The problem seems to be that scanner() calls recursively one extra
> time when looking at "notadirectory/".

(You can easily see this in with the above change with "print */" in a
directory with some non-directory files.)

I don't see any evidence that's either new or a bug --- it's just the
rather icky way "*/" works.  The scanner doesn't know in advance the
"/" is the end, so it attempts to go down to the next level (hence the
recursion), where it adds an empty string as it has nothing else to
add.  Later it (actually, I think, a library test) sees the "/" is
at the end and prunes non-directory files so you get the right answer.

This isn't actually specific to this case.  Just to show this up (it's
not useful for any other purpose), try the patch at the bottom and then
e.g. (if you're in the Src directory)

print */zle_tricky.c

You'll see lots of messages like

Recursing to add zle_tricky.c to zsh.ico/
Recursing to add zle_tricky.c to options.c/
Recursing to add zle_tricky.c to subst.c/
Recursing to add zle_tricky.c to signals.pro/

plus the right answer at the end, Zle/zle_tricky.c.

If you missed out zle_tricky.c you'd get your case --- it's
got to the "/" and discovered there's no glob after it, so it simply
adds the string and sees if the file exists.  If it's not a directory,
the file doesn't exist.  The only difference in the case of a trailing
"/" is that because you've only added a null string, then the file
"foo/" exists if and only if foo is a directory --- as long as the
system test works like that, which is done in statfullpath(), which I
didn't get around to walking through so if you want to be quite sure you
can go do that.

I expect you could truncate it earlier with the right test --- I
certainly hadn't realised/remembered this is how a trailing "/" worked,
I'd assumed it was internally treated as (/).

pws

diff --git a/Src/glob.c b/Src/glob.c
index 2051016..add1406 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -463,6 +463,8 @@ insert(char *s, int checked)
  * with successive bits of the path until we've    *
  * tried all of it.                                */
 
+static int recursing = -1;
+
 /**/
 static void
 scanner(Complist q, int shortcircuit)
@@ -477,14 +479,18 @@ scanner(Complist q, int shortcircuit)
 	return;
     init_dirsav(&ds);
 
+    recursing++;
+
     if ((closure = q->closure)) {
 	/* (foo/)# - match zero or more dirs */
 	if (q->closure == 2)	/* (foo/)## - match one or more dirs */
 	    q->closure = 1;
 	else {
 	    scanner(q->next, shortcircuit);
-	    if (shortcircuit && shortcircuit == matchct)
+	    if (shortcircuit && shortcircuit == matchct) {
+		recursing--;
 		return;
+	    }
 	}
     }
     p = q->pat;
@@ -499,13 +505,18 @@ scanner(Complist q, int shortcircuit)
 	if (l + !l + pathpos - pathbufcwd >= PATH_MAX) {
 	    int err;
 
-	    if (l >= PATH_MAX)
+	    if (l >= PATH_MAX) {
+		recursing--;
 		return;
+	    }
 	    err = lchdir(unmeta(pathbuf + pathbufcwd), &ds, 0);
-	    if (err == -1)
+	    if (err == -1) {
+		recursing--;
 		return;
+	    }
 	    if (err) {
 		zerr("current directory lost during glob");
+		recursing--;
 		return;
 	    }
 	    pathbufcwd = pathpos;
@@ -532,8 +543,10 @@ scanner(Complist q, int shortcircuit)
 		    addpath(str, l);
 		    if (!closure || !statfullpath("", NULL, 1)) {
 			scanner((q->closure) ? q : q->next, shortcircuit);
-			if (shortcircuit && shortcircuit == matchct)
+			if (shortcircuit && shortcircuit == matchct) {
+			    recursing--;
 			    return;
+			}
 		    }
 		    pathbuf[pathpos = oppos] = '\0';
 		}
@@ -541,9 +554,13 @@ scanner(Complist q, int shortcircuit)
 	} else {
 	    if (str[l])
 		str = dupstrpfx(str, l);
+	    if (recursing)
+		fprintf(stderr, "Recursing to add %s to %s\n", str, pathbuf);
 	    insert(str, 0);
-	    if (shortcircuit && shortcircuit == matchct)
+	    if (shortcircuit && shortcircuit == matchct) {
+		recursing--;
 		return;
+	    }
 	}
     } else {
 	/* Do pattern matching on current path section. */
@@ -553,8 +570,10 @@ scanner(Complist q, int shortcircuit)
 	char *subdirs = NULL;
 	int subdirlen = 0;
 
-	if (lock == NULL)
+	if (lock == NULL) {
+	    recursing--;
 	    return;
+	}
 	while ((fn = zreaddir(lock, 1)) && !errflag) {
 	    /* prefix and suffix are zle trickery */
 	    if (!dirs && !colonmod &&
@@ -636,6 +655,7 @@ scanner(Complist q, int shortcircuit)
 		    insert(fn, 1);
 		    if (shortcircuit && shortcircuit == matchct) {
 			closedir(lock);
+			recursing--;
 			return;
 		    }
 		}
@@ -653,8 +673,10 @@ scanner(Complist q, int shortcircuit)
 		fn += sizeof(int);
 		/* scan next level */
 		scanner((q->closure) ? q : q->next, shortcircuit); 
-		if (shortcircuit && shortcircuit == matchct)
+		if (shortcircuit && shortcircuit == matchct) {
+		    recursing--;
 		    return;
+		}
 		pathbuf[pathpos = oppos] = '\0';
 	    }
 	    hrealloc(subdirs, subdirlen, 0);
@@ -668,6 +690,7 @@ scanner(Complist q, int shortcircuit)
 	    close(ds.dirfd);
 	pathbufcwd = pbcwdsav;
     }
+    recursing--;
     return;
 }