* Re: [PATCH] isearch: do not use PAT_STATIC since we call zle hooks
2017-01-28 19:47 ` Peter Stephenson
@ 2017-01-29 0:01 ` Bart Schaefer
2017-01-29 16:24 ` Bart Schaefer
0 siblings, 1 reply; 13+ messages in thread
From: Bart Schaefer @ 2017-01-29 0:01 UTC (permalink / raw)
To: zsh-workers
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);
^ permalink raw reply [flat|nested] 13+ messages in thread