Re: -pcre-match doesn't work since version 5.4.1

Re: -pcre-match doesn't work since version 5.4.1

[Phil Pennock]

[ SWITCHED TO -workers ]

On 2017-08-10 at 21:07 -0400, Phil Pennock wrote:
> On 2017-08-11 at 00:29 +0200, nimaje+zml@bureaucracy.de wrote:
> > after upgrading to version 5.4.1 I noticed that using -pcre-match doesn't
> > work
> > 
> > % zmodload zsh/pcre; [[ 'dxd' -pcre-match ^d+$ ]]
> > 
> > zsh: unknown condition: -pcre-match
> 
> I see this too; none of the loadable infix operators seem to be working,
> so it's not specific to the PCRE module.  -regex-match doesn't work
> either.

No fix yet, still investigating the commit which broke.

With a git bisect script (below) and:
  git bisect start HEAD zsh-5.3.1 --
  git bisect run ../bisect-script
git spits out that:

------------------------8< git bisect result >8-------------------------
f3f8537cfa05414ad14494e809d9ebfeef86ebbc is the first bad commit
commit f3f8537cfa05414ad14494e809d9ebfeef86ebbc
Author: Peter Stephenson <pws@zsh.org>
Date:   Tue Mar 7 10:43:58 2017 +0000

    40760: Always tokenize unquoted - to Dash.

    This fixes use of pattern match character ranges in unusual contexts.

    Attempt to detect a tokenized - in cases where we don't care.
------------------------8< git bisect result >8-------------------------

-------------------------8< ../bisect-script >8-------------------------
#!/bin/sh -eu
make
ln -nsf . Src/Modules/zsh
./Src/zsh -fc '
module_path=( /Users/pdp/src/zsh/code/Src/Modules )
zmodload zsh/pcre
[[ foo -pcre-match ^f..$ ]]
'
-------------------------8< ../bisect-script >8-------------------------

Re: -pcre-match doesn't work since version 5.4.1

[Bart Schaefer]

[-- Attachment #1: Type: text/plain, Size: 218 bytes --]

On Aug 10, 2017 6:37 PM, "Phil Pennock" <
zsh-workers+phil.pennock@spodhuis.org> wrote:


    40760: Always tokenize unquoted - to Dash.


I remember this, and thought there was a subsequent patch that fixed it
again.

[PATCH] repair conds where - within cond name

[Phil Pennock]

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

On 2017-08-10 at 18:49 -0700, Bart Schaefer wrote:
> On Aug 10, 2017 6:37 PM, "Phil Pennock" <
> zsh-workers+phil.pennock@spodhuis.org> wrote:
> 
>     40760: Always tokenize unquoted - to Dash.
> 
> I remember this, and thought there was a subsequent patch that fixed it
> again.

Not in master.

Fix below; adds a test too.  Did I really not provide tests for the
zsh/regex module?  :(

-Phil

From 6f0dcb270ff13327fef341974a1dde4e9a7a4f86 Mon Sep 17 00:00:00 2001
From: Phil Pennock <phil@pennock-tech.com>
Subject: [PATCH] repair conds where - within cond name
Date: Thu, 10 Aug 2017 21:58:11 -0400

In 40760 we switched to tokenizing `-` to `Dash` broadly; the condtab
lookup is a simple strcmp but some of the loadable conditionals have a
`-` within their name (-regex-match and -pcre-match).  That failed.

De-tokenize this one character before lookup.

Add a test-case.
---
 Src/module.c      | 17 +++++++++++++++--
 Test/V07pcre.ztst |  9 +++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Src/module.c b/Src/module.c
index 21d68b1ac..b5f522bf0 100644
--- a/Src/module.c
+++ b/Src/module.c
@@ -649,11 +649,21 @@ getconddef(int inf, const char *name, int autol)
 {
     Conddef p;
     int f = 1;
+    char *lookup, *s;
+
+    /* detokenize the Dash to the form encoded in lookup tables */
+    lookup = ztrdup(name);
+    if (!lookup)
+	return NULL;
+    for (s = lookup; *s != '\0'; s++) {
+	if (*s == Dash)
+	    *s = '-';
+    }
 
     do {
 	for (p = condtab; p; p = p->next) {
 	    if ((!!inf == !!(p->flags & CONDF_INFIX)) &&
-		!strcmp(name, p->name))
+		!strcmp(lookup, p->name))
 		break;
 	}
 	if (autol && p && p->module) {
@@ -664,16 +674,19 @@ getconddef(int inf, const char *name, int autol)
 	    if (f) {
 		(void)ensurefeature(p->module,
 				    (p->flags & CONDF_INFIX) ? "C:" : "c:",
-				    (p->flags & CONDF_AUTOALL) ? NULL : name);
+				    (p->flags & CONDF_AUTOALL) ? NULL : lookup);
 		f = 0;
 		p = NULL;
 	    } else {
 		deleteconddef(p);
+		free(lookup);
 		return NULL;
 	    }
 	} else
 	    break;
     } while (!p);
+
+    free(lookup);
     return p;
 }
 
diff --git a/Test/V07pcre.ztst b/Test/V07pcre.ztst
index 7426e7bf8..ab41d33dc 100644
--- a/Test/V07pcre.ztst
+++ b/Test/V07pcre.ztst
@@ -137,6 +137,15 @@
 0:ensure ASCII NUL passes in and out of matched plaintext
 >6; 3; 3
 
+# Ensure the long-form infix operator works
+  [[ foo -pcre-match ^f..$ ]]
+  print $?
+  [[ foo -pcre-match ^g..$ ]]
+  print $?
+0:infix -pcre-match works
+>0
+>1
+
 # Subshell because crash on failure
   ( setopt re_match_pcre
     [[ test.txt =~ '^(.*_)?(test)' ]]
-- 
2.14.1


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 996 bytes --]

Re: [PATCH] repair conds where - within cond name

[Peter Stephenson]

On Thu, 10 Aug 2017 22:02:46 -0400
Phil Pennock <zsh-workers+phil.pennock@spodhuis.org> wrote:
> On 2017-08-10 at 18:49 -0700, Bart Schaefer wrote:
> > On Aug 10, 2017 6:37 PM, "Phil Pennock" <
> > zsh-workers+phil.pennock@spodhuis.org> wrote:
> > 
> >     40760: Always tokenize unquoted - to Dash.
> > 
> > I remember this, and thought there was a subsequent patch that fixed it
> > again.
> 
> Not in master.
> 
> Fix below; adds a test too.  Did I really not provide tests for the
> zsh/regex module?  :(

Thanks for that...

> +    lookup = ztrdup(name);

I'd be inclined just to put this on the heap.  I can do that myself if
that's OK.

pws

Re: [PATCH] repair conds where - within cond name

[Phil Pennock]

On 2017-08-11 at 09:31 +0100, Peter Stephenson wrote:
> On Thu, 10 Aug 2017 22:02:46 -0400
> Phil Pennock <zsh-workers+phil.pennock@spodhuis.org> wrote:
> > +    lookup = ztrdup(name);
> 
> I'd be inclined just to put this on the heap.  I can do that myself if
> that's OK.

I'm guessing you meant stack, or some short-lived pool.  I couldn't
remember the means to do that in zsh and brief looking didn't help, so I
went with what worked.

"Yes" is the answer.  Go ahead.

I could switch a bunch of memory stuff in the PCRE/regex libs over to
the same too, at the same time as adding tests, if I manage to get some
spare time.  This one is nagging at me right now, so it might happen.

-Phil

Re: [PATCH] repair conds where - within cond name

[Daniel Shahaf]

Phil Pennock wrote on Fri, 11 Aug 2017 13:18 -0400:
> On 2017-08-11 at 09:31 +0100, Peter Stephenson wrote:
> > On Thu, 10 Aug 2017 22:02:46 -0400
> > Phil Pennock <zsh-workers+phil.pennock@spodhuis.org> wrote:
> > > +    lookup = ztrdup(name);
> > 
> > I'd be inclined just to put this on the heap.  I can do that myself if
> > that's OK.
> 
> I'm guessing you meant stack, or some short-lived pool.  I couldn't
> remember the means to do that in zsh and brief looking didn't help, so I
> went with what worked.

The "heap" in this context is pools, yes.  See comment at the top
of Src/mem.c