Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: [PATCH] isearch: do not use PAT_STATIC since we call zle hooks



On Jan 28,  7:47pm, Peter Stephenson wrote:
}
} But my gut feel is compctl only ever got to the point that it was doing
} simple local stuff that finished quickly, and we're morally entitled to
} assume that anything that isn't like that is using compsys.  So I'd be
} inclined to try what you suggest and just make sure a few simple builtin
} completions with compctl work.

OK, with that confirmed ... here's a largish patch.

It's somewhat arbitrary whether to change the PAT_* flag or to apply
signal queuing.  For example the patch that started all this (40285)
uses PAT_ZDUP, but it would be possible to allow the pattern to be
placed on the heap every time instead.  However this is in a loop,
and the pattern may change multiple times which could lead to several
patterns piling up on the heap before we finally finish the loop and
clean up.  So I elected not to leave that as it was, though I updated
some comments.

In other cases it was obvious that global state was being changed so
it seemed better to add, or to widen the scope of, queue_signals().



diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 0ef7539..2c87be1 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -544,7 +544,8 @@ ptyread(char *nam, Ptycmd cmd, char **args, int noblock, int mustmatch)
 	p = dupstring(args[1]);
 	tokenize(p);
 	remnulargs(p);
-	if (!(prog = patcompile(p, PAT_STATIC, NULL))) {
+	/* Signals handlers might stomp PAT_STATIC */
+	if (!(prog = patcompile(p, PAT_ZDUP, NULL))) {
 	    zwarnnam(nam, "bad pattern: %s", args[1]);
 	    return 1;
 	}
@@ -682,9 +683,14 @@ ptyread(char *nam, Ptycmd cmd, char **args, int noblock, int mustmatch)
 	write_loop(1, buf, used);
     }
 
-    if (seen && (!prog || matchok || !mustmatch))
-	return 0;
-    return cmd->fin + 1;
+    {
+	int ret = cmd->fin + 1;
+	if (seen && (!prog || matchok || !mustmatch))
+	    ret = 0;
+	if (prog)
+	    freepatprog(prog);
+	return ret;
+    }
 }
 
 static int
diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index d95c0c2..19a8306 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -510,25 +510,33 @@ bin_zstyle(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    zwarnnam(nam, "too many arguments");
 	    return 1;
 	}
+
+	queue_signals();	/* Protect PAT_STATIC */
+
 	if (context) {
 	    tokenize(context);
 	    zstyle_contprog = patcompile(context, PAT_STATIC, NULL);
 
-	    if (!zstyle_contprog)
+	    if (!zstyle_contprog) {
+		unqueue_signals();
 		return 1;
+	    }
 	} else
 	    zstyle_contprog = NULL;
 
 	if (stylename) {
 	    s = (Style)zstyletab->getnode2(zstyletab, stylename);
-	    if (!s)
+	    if (!s) {
+		unqueue_signals();
 		return 1;
+	    }
 	    zstyletab->printnode(&s->node, list);
 	} else {
 	    scanhashtable(zstyletab, 1, 0, 0,
 			  zstyletab->printnode, list);
 	}
 
+	unqueue_signals();
 	return 0;
     }
     switch (args[0][1]) {
@@ -675,14 +683,20 @@ bin_zstyle(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    char **vals;
 	    Patprog prog;
 
+	    queue_signals();	/* Protect PAT_STATIC */
+
 	    tokenize(args[3]);
 
 	    if ((vals = lookupstyle(args[1], args[2])) &&
 		(prog = patcompile(args[3], PAT_STATIC, NULL))) {
 		while (*vals)
-		    if (pattry(prog, *vals++))
+		    if (pattry(prog, *vals++)) {
+			unqueue_signals();
 			return 0;
+		    }
 	    }
+
+	    unqueue_signals();
 	    return 1;
 	}
 	break;
diff --git a/Src/Zle/compctl.c b/Src/Zle/compctl.c
index 52c6f12..9e6ccb4 100644
--- a/Src/Zle/compctl.c
+++ b/Src/Zle/compctl.c
@@ -99,7 +99,7 @@ freecompctlp(HashNode hn)
 }
 
 /**/
-void
+static void
 freecompctl(Compctl cc)
 {
     if (cc == &cc_default ||
@@ -142,7 +142,7 @@ freecompctl(Compctl cc)
 }
 
 /**/
-void
+static void
 freecompcond(void *a)
 {
     Compcond cc = (Compcond) a;
@@ -186,7 +186,7 @@ freecompcond(void *a)
 }
 
 /**/
-int
+static int
 compctlread(char *name, char **args, Options ops, char *reply)
 {
     char *buf, *bptr;
@@ -1564,6 +1564,8 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
     Compctl cc = NULL;
     int ret = 0;
 
+    queue_signals();
+
     /* clear static flags */
     cclist = 0;
     showmask = 0;
@@ -1571,12 +1573,15 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
     /* Parse all the arguments */
     if (*argv) {
 	/* Let's see if this is a global matcher definition. */
-	if ((ret = get_gmatcher(name, argv)))
+	if ((ret = get_gmatcher(name, argv))) {
+	    unqueue_signals();
 	    return ret - 1;
+	}
 
 	cc = (Compctl) zshcalloc(sizeof(*cc));
 	if (get_compctl(name, &argv, cc, 1, 0, 0)) {
 	    freecompctl(cc);
+	    unqueue_signals();
 	    return 1;
 	}
 
@@ -1604,6 +1609,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	printcompctl((cclist & COMP_LIST) ? "" : "DEFAULT", &cc_default, 0, 0);
  	printcompctl((cclist & COMP_LIST) ? "" : "FIRST", &cc_first, 0, 0);
 	print_gmatcher((cclist & COMP_LIST));
+	unqueue_signals();
 	return ret;
     }
 
@@ -1642,6 +1648,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	    printcompctl("", &cc_first, 0, 0);
 	if (cclist & COMP_LISTMATCH)
 	    print_gmatcher(COMP_LIST);
+	unqueue_signals();
 	return ret;
     }
 
@@ -1656,6 +1663,7 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	    compctl_process_cc(argv, cc);
     }
 
+    unqueue_signals();
     return ret;
 }
 
@@ -1667,12 +1675,18 @@ bin_compctl(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 static int
 bin_compcall(char *name, UNUSED(char **argv), Options ops, UNUSED(int func))
 {
+    int ret;
+
     if (incompfunc != 1) {
 	zwarnnam(name, "can only be called from completion function");
 	return 1;
     }
-    return makecomplistctl((OPT_ISSET(ops,'T') ? 0 : CFN_FIRST) |
-			   (OPT_ISSET(ops,'D') ? 0 : CFN_DEFAULT));
+
+    queue_signals();
+    ret = makecomplistctl((OPT_ISSET(ops,'T') ? 0 : CFN_FIRST) |
+			  (OPT_ISSET(ops,'D') ? 0 : CFN_DEFAULT));
+    unqueue_signals();
+    return ret;
 }
 
 /*
@@ -1756,6 +1770,8 @@ ccmakehookfn(UNUSED(Hookdef dummy), struct ccmakedat *dat)
     int onm = nmatches, odm = diffmatches, osi = movefd(0);
     LinkNode n;
 
+    queue_signals();
+
     /* We build a copy of the list of matchers to use to make sure that this
      * works even if a shell function called from the completion code changes
      * the global matchers. */
@@ -1883,6 +1899,8 @@ ccmakehookfn(UNUSED(Hookdef dummy), struct ccmakedat *dat)
     }
     redup(osi, 0);
     dat->lst = 1;
+
+    unqueue_signals();
     return 0;
 }
 
@@ -2044,7 +2062,7 @@ maketildelist(void)
 /* This does the check for compctl -x `n' and `N' patterns. */
 
 /**/
-int
+static int
 getcpat(char *str, int cpatindex, char *cpat, int class)
 {
     char *s, *t, *p;
diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 48fcd47..49b338f 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -896,6 +896,8 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod)
 	    int i, l = arrlen(compwords), t = 0, b = 0, e = l - 1;
 	    Patprog pp;
 
+	    queue_signals();	/* Protect PAT_STATIC */
+
 	    i = compcurrent - 1;
 	    if (i < 0 || i >= l)
 		return 0;
@@ -930,6 +932,9 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod)
 		t = 0;
 	    if (t && mod)
 		restrict_range(b, e);
+
+	    unqueue_signals();
+
 	    return t;
 	}
     case CVT_PRENUM:
@@ -952,6 +957,8 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod)
 	{
 	    Patprog pp;
 
+	    queue_signals();	/* Protect PAT_STATIC */
+
 	    if (!na)
 		return 0;
 
@@ -1036,6 +1043,9 @@ do_comp_vars(int test, int na, char *sa, int nb, char *sb, int mod)
 		if (mod)
 		    ignore_suffix(ol - (p - compsuffix));
 	    }
+
+	    unqueue_signals();
+
 	    return 1;
 	}
     }
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 5b9ceec..325da6d 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -3928,6 +3928,8 @@ bin_comptry(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 		    if (*q) {
 			char *qq, *qqq;
 
+			queue_signals();
+
 			if (c)
 			    *c = '\0';
 
@@ -3999,6 +4001,8 @@ bin_comptry(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 			}
 			if (c)
 			    *c = ':';
+
+			unqueue_signals();
 		    }
 		}
 		if (num) {
@@ -4708,6 +4712,8 @@ cfp_add_sdirs(LinkList final, LinkList orig, char *skipped,
 		if (!*p)
 		    continue;
 
+		queue_signals();	/* Protect PAT_STATIC */
+
 		tokenize(f);
 		pprog = patcompile(f, PAT_STATIC, NULL);
 		untokenize(f);
@@ -4740,6 +4746,8 @@ cfp_add_sdirs(LinkList final, LinkList orig, char *skipped,
 			}
 		    }
 		}
+
+		unqueue_signals();
 	    }
 	}
     }
diff --git a/Src/Zle/zle_hist.c b/Src/Zle/zle_hist.c
index 434735d..581ca49 100644
--- a/Src/Zle/zle_hist.c
+++ b/Src/Zle/zle_hist.c
@@ -1220,10 +1220,12 @@ doisearch(char **args, int dir, int pattern)
 		char *patbuf = ztrdup(sbuf);
 		char *patstring;
 		/*
-		 * Do not use static pattern buffer (PAT_STATIC) since we call zle hooks,
-		 * which might call other pattern functions. Use PAT_ZDUP instead.
-		 * Use PAT_NOANCH because we don't need the match
-		 * anchored to the end, even if it is at the start.
+		 * Do not use static pattern buffer (PAT_STATIC) since we
+		 * call zle hooks, which might call other pattern
+		 * functions.  Use PAT_ZDUP because we re-use the pattern
+		 * in subsequent loops, so we can't pushheap/popheap.
+		 * Use PAT_NOANCH because we don't need the match anchored
+		 * to the end, even if it is at the start.
 		 */
 		int patflags = PAT_ZDUP|PAT_NOANCH;
 		if (sbuf[0] == '^') {
diff --git a/Src/builtin.c b/Src/builtin.c
index 219fbc9..394d206 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -539,18 +539,18 @@ bin_enable(char *name, char **argv, Options ops, int func)
     /* With -m option, treat arguments as glob patterns. */
     if (OPT_ISSET(ops,'m')) {
 	for (; *argv; argv++) {
+	    queue_signals();
+
 	    /* parse pattern */
 	    tokenize(*argv);
-	    if ((pprog = patcompile(*argv, PAT_STATIC, 0))) {
-		queue_signals();
+	    if ((pprog = patcompile(*argv, PAT_STATIC, 0)))
 		match += scanmatchtable(ht, pprog, 0, 0, 0, scanfunc, 0);
-		unqueue_signals();
-	    }
 	    else {
 		untokenize(*argv);
 		zwarnnam(name, "bad pattern : %s", *argv);
 		returnval = 1;
 	    }
+	    unqueue_signals();
 	}
 	/* If we didn't match anything, we return 1. */
 	if (!match)
@@ -3136,9 +3136,9 @@ bin_functions(char *name, char **argv, Options ops, int func)
 	} else if (OPT_ISSET(ops,'m')) {
 	    /* List matching functions. */
 	    for (; *argv; argv++) {
+		queue_signals();
 		tokenize(*argv);
 		if ((pprog = patcompile(*argv, PAT_STATIC, 0))) {
-		    queue_signals();
 		    for (p = mathfuncs, q = NULL; p; q = p) {
 			MathFunc next;
 			do {
@@ -3157,12 +3157,12 @@ bin_functions(char *name, char **argv, Options ops, int func)
 			if (p)
 			    p = p->next;
 		    }
-		    unqueue_signals();
 		} else {
 		    untokenize(*argv);
 		    zwarnnam(name, "bad pattern : %s", *argv);
 		    returnval = 1;
 		}
+		unqueue_signals();
 	    }
 	} else if (OPT_PLUS(ops,'M')) {
 	    /* Delete functions. -m is allowed but is handled above. */
@@ -3312,11 +3312,11 @@ bin_functions(char *name, char **argv, Options ops, int func)
     if (OPT_ISSET(ops,'m')) {
 	on &= ~PM_UNDEFINED;
 	for (; *argv; argv++) {
+	    queue_signals();
 	    /* expand argument */
 	    tokenize(*argv);
 	    if ((pprog = patcompile(*argv, PAT_STATIC, 0))) {
 		/* with no options, just print all functions matching the glob pattern */
-		queue_signals();
 		if (!(on|off) && !OPT_ISSET(ops,'X')) {
 		    scanmatchshfunc(pprog, 1, 0, DISABLED,
 				   shfunctab->printnode, pflags, expand);
@@ -3336,12 +3336,12 @@ bin_functions(char *name, char **argv, Options ops, int func)
 			    }
 		    }
 		}
-		unqueue_signals();
 	    } else {
 		untokenize(*argv);
 		zwarnnam(name, "bad pattern : %s", *argv);
 		returnval = 1;
 	    }
+	    unqueue_signals();
 	}
 	return returnval;
     }
@@ -3469,11 +3469,11 @@ bin_unset(char *name, char **argv, Options ops, int func)
     /* with -m option, treat arguments as glob patterns */
     if (OPT_ISSET(ops,'m')) {
 	while ((s = *argv++)) {
+	    queue_signals();
 	    /* expand */
 	    tokenize(s);
 	    if ((pprog = patcompile(s, PAT_STATIC, NULL))) {
 		/* Go through the parameter table, and unset any matches */
-		queue_signals();
 		for (i = 0; i < paramtab->hsize; i++) {
 		    for (pm = (Param) paramtab->nodes[i]; pm; pm = next) {
 			/* record pointer to next, since we may free this one */
@@ -3486,12 +3486,12 @@ bin_unset(char *name, char **argv, Options ops, int func)
 			}
 		    }
 		}
-		unqueue_signals();
 	    } else {
 		untokenize(s);
 		zwarnnam(name, "bad pattern : %s", s);
 		returnval = 1;
 	    }
+	    unqueue_signals();
 	}
 	/* If we didn't match anything, we return 1. */
 	if (!match)
@@ -3655,6 +3655,7 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    pushheap();
 	    matchednodes = newlinklist();
 	}
+	queue_signals();
 	for (; *argv; argv++) {
 	    /* parse the pattern */
 	    tokenize(*argv);
@@ -3664,7 +3665,6 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 		returnval = 1;
 		continue;
 	    }
-	    queue_signals();
 	    if (!OPT_ISSET(ops,'p')) {
 		/* -p option is for path search only.    *
 		 * We're not using it, so search for ... */
@@ -3695,9 +3695,9 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	    scanmatchtable(cmdnamtab, pprog, 1, 0, 0,
 			   (all ? fetchcmdnamnode : cmdnamtab->printnode),
 			   printflags);
-
-	    unqueue_signals();
+	    run_queued_signals();
 	}
+	unqueue_signals();
 	if (all) {
 	    allmatched = argv = zlinklist2array(matchednodes);
 	    matchednodes = NULL;
@@ -4024,11 +4024,11 @@ bin_unhash(char *name, char **argv, Options ops, int func)
      * "unhash -m '*'" is legal, but not recommended.    */
     if (OPT_ISSET(ops,'m')) {
 	for (; *argv; argv++) {
+	    queue_signals();
 	    /* expand argument */
 	    tokenize(*argv);
 	    if ((pprog = patcompile(*argv, PAT_STATIC, NULL))) {
 		/* remove all nodes matching glob pattern */
-		queue_signals();
 		for (i = 0; i < ht->hsize; i++) {
 		    for (hn = ht->nodes[i]; hn; hn = nhn) {
 			/* record pointer to next, since we may free this one */
@@ -4039,12 +4039,12 @@ bin_unhash(char *name, char **argv, Options ops, int func)
 			}
 		    }
 		}
-		unqueue_signals();
 	    } else {
 		untokenize(*argv);
 		zwarnnam(name, "bad pattern : %s", *argv);
 		returnval = 1;
 	    }
+	    unqueue_signals();
 	}
 	/* If we didn't match anything, we return 1. */
 	if (!match)
@@ -4127,18 +4127,18 @@ bin_alias(char *name, char **argv, Options ops, UNUSED(int func))
      * glob patterns of aliases to display.       */
     if (OPT_ISSET(ops,'m')) {
 	for (; *argv; argv++) {
+	    queue_signals();
 	    tokenize(*argv);  /* expand argument */
 	    if ((pprog = patcompile(*argv, PAT_STATIC, NULL))) {
 		/* display the matching aliases */
-		queue_signals();
 		scanmatchtable(ht, pprog, 1, flags1, flags2,
 			       ht->printnode, printflags);
-		unqueue_signals();
 	    } else {
 		untokenize(*argv);
 		zwarnnam(name, "bad pattern : %s", *argv);
 		returnval = 1;
 	    }
+	    unqueue_signals();
 	}
 	return returnval;
     }
@@ -4348,10 +4348,12 @@ bin_print(char *name, char **args, Options ops, int func)
 	    zwarnnam(name, "no pattern specified");
 	    return 1;
 	}
+	queue_signals();
 	tokenize(*args);
 	if (!(pprog = patcompile(*args, PAT_STATIC, NULL))) {
 	    untokenize(*args);
 	    zwarnnam(name, "bad pattern: %s", *args);
+	    unqueue_signals();
 	    return 1;
 	}
 	for (t = p = ++args; *p; p++)
@@ -4359,6 +4361,7 @@ bin_print(char *name, char **args, Options ops, int func)
 		*t++ = *p;
 	*t = NULL;
 	first = args;
+	unqueue_signals();
 	if (fmt && !*args) return 0;
     }
     /* compute lengths, and interpret according to -P, -D, -e, etc. */
diff --git a/Src/cond.c b/Src/cond.c
index 42e9de3..8ab0193 100644
--- a/Src/cond.c
+++ b/Src/cond.c
@@ -295,6 +295,8 @@ evalcond(Estate state, char *fromtest)
 	    int test, npat = state->pc[1];
 	    Patprog pprog = state->prog->pats[npat];
 
+	    queue_signals();
+
 	    if (pprog == dummy_patprog1 || pprog == dummy_patprog2) {
 		char *opat;
 		int save;
@@ -308,6 +310,7 @@ evalcond(Estate state, char *fromtest)
 		if (!(pprog = patcompile(right, (save ? PAT_ZDUP : PAT_STATIC),
 					 NULL))) {
 		    zwarnnam(fromtest, "bad pattern: %s", right);
+		    unqueue_signals();
 		    return 2;
 		}
 		else if (save)
@@ -316,6 +319,8 @@ evalcond(Estate state, char *fromtest)
 	    state->pc += 2;
 	    test = (pprog && pattry(pprog, left));
 
+	    unqueue_signals();
+
 	    return !(ctype == COND_STRNEQ ? !test : test);
 	}
     case COND_STRLT:
diff --git a/Src/glob.c b/Src/glob.c
index 623e6f1..ff6b258 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -2462,13 +2462,20 @@ xpandbraces(LinkList list, LinkNode *np)
 int
 matchpat(char *a, char *b)
 {
-    Patprog p = patcompile(b, PAT_STATIC, NULL);
+    Patprog p;
+    int ret;
 
-    if (!p) {
+    queue_signals();	/* Protect PAT_STATIC */
+
+    if (!(p = patcompile(b, PAT_STATIC, NULL))) {
 	zerr("bad pattern: %s", b);
-	return 0;
-    }
-    return pattry(p, a);
+	ret = 0;
+    } else
+      ret = pattry(p, a);
+
+    unqueue_signals();
+
+    return ret;
 }
 
 /* do the ${foo%%bar}, ${foo#bar} stuff */
diff --git a/Src/loop.c b/Src/loop.c
index ae87b2f..f7eae30 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -620,7 +620,9 @@ execcase(Estate state, int do_exec)
 	    spprog = state->prog->pats + npat;
 	    pprog = NULL;
 	    pat = NULL;
-	
+
+	    queue_signals();
+
 	    if (isset(XTRACE)) {
 		int htok = 0;
 		pat = dupstring(ecrawstr(state->prog, state->pc, &htok));
@@ -657,6 +659,8 @@ execcase(Estate state, int do_exec)
 		patok = anypatok = 1;
 	    state->pc += 2;
 	    nalts--;
+
+	    unqueue_signals();
 	}
 	state->pc += 2 * nalts;
 	if (isset(XTRACE)) {
diff --git a/Src/options.c b/Src/options.c
index e0b67d2..2b5795b 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -647,7 +647,7 @@ bin_setopt(char *nam, char **args, UNUSED(Options ops), int isun)
 
 	    /* Expand the current arg. */
 	    tokenize(s);
-	    if (!(pprog = patcompile(s, PAT_STATIC, NULL))) {
+	    if (!(pprog = patcompile(s, PAT_HEAPDUP, NULL))) {
 		zwarnnam(nam, "bad pattern: %s", *args);
 		continue;
 	    }
diff --git a/Src/parse.c b/Src/parse.c
index 314cc09..699ea49 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -3413,6 +3413,7 @@ build_cur_dump(char *nam, char *dump, char **names, int match, int map,
 
 	for (; *names; names++) {
 	    tokenize(pat = dupstring(*names));
+	    /* Signal-safe here, caller queues signals */
 	    if (!(pprog = patcompile(pat, PAT_STATIC, NULL))) {
 		zwarnnam(nam, "bad pattern: %s", *names);
 		close(dfd);



Messages sorted by: Reverse Date, Date, Thread, Author