Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 X-AuditID: cbfec7f5-f792a6d000001302-c7-5783702d3e2e Date: Mon, 11 Jul 2016 11:08:42 +0100 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: [BUG] Directory glob picks up running or already-run scripts on OS X Message-id: <20160711110842.0315fcda@pwslap01u.europe.root.pri> In-reply-to: <160710215821.ZM11734@torch.brasslantern.com> References: <6ED44F4D-7D51-4F0B-935D-4A3868E0C5C3@gmail.com> <160710215821.ZM11734@torch.brasslantern.com> Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrILMWRmVeSWpSXmKPExsVy+t/xq7q6Bc3hBh2PpC0ONj9kcmD0WHXw A1MAYxSXTUpqTmZZapG+XQJXxqr1j5kLPulWPN0wn62B8aNiFyMnh4SAicS6N4cZIWwxiQv3 1rN1MXJxCAksZZS4+mwKC4Qzg0li6sI17CBVQgLnGCUmP9SCSJxllNh26CRYgkVAVaJ12kUW EJtNwFBi6qbZYGNFBMQlzq49DxYXFgiWuPyxDyzOK2Av8fDDM2YQm1PASmLL83XMEEMfMEl8 3v2BDSTBL6AvcfXvJyaI++wlZl45A9UsKPFj8j2wocwCWhKbtzWxQtjyEpvXvGWGuFRd4sbd 3ewTGIVnIWmZhaRlFpKWBYzMqxhFU0uTC4qT0nON9IoTc4tL89L1kvNzNzFCAvrrDsalx6wO MQpwMCrx8AZsbwoXYk0sK67MPcQowcGsJML7Mqs5XIg3JbGyKrUoP76oNCe1+BCjNAeLkjjv zF3vQ4QE0hNLUrNTUwtSi2CyTBycUg2MCVqb06xi3CUD4kJnTnB4vIfzl4lQ1MJQ/z63trZJ O1cYrO58f7EjdcYRxZrP3y+GnlrOwqtcuvaAvu/uJdcMsrWsF51zEK+perHXVDfRaW97xEHf dYcv7+U3tNA+l+mspvto3i6+rr1znpdnxpYoyD8vmsx2Z7rgG4Xt92cxiR7o2LVG/tR6JZbi jERDLeai4kQAkvwU82QCAAA= X-Seq: zsh-workers 38824 On Sun, 10 Jul 2016 21:58:21 -0700 Bart Schaefer 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; }