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

PATCH: functions with redirections



This seems to add the ability to define functions with redirections that
take effect when the function is run.  Parsing didn't actually need
changing at all, despite what I said a few days ago --- we already parse
the redirections, it's just a question of saving them at the definition
and digging them back out when executing.

I haven't yet looked at dump files.  That's partly becuase I don't
understand them and partly because I don't use them.  Any hints would be
great.

If anyone wants something else to do they can think about whether I've
introduced any subtle memory leaks.

diff --git a/NEWS b/NEWS
index 7dc134e..bf8969b 100644
--- a/NEWS
+++ b/NEWS
@@ -107,6 +107,13 @@ Changes since 5.0.0
   Also, the value of $pipestatus is now updated when a job stops, not just
   when it exits.
 
+- Redirections applied to function definitions take effect when the
+  function is executed, not when it is defined.  Other shells already
+  work this way.  For example,
+    fn() { echo hello } >~/logfile
+  Running fn writes "hello" to logfile.  In older versions of the shell
+  it would create an empty file at the point of definition.
+
 Changes between 4.2 and 5.0.0
 -----------------------------
 
diff --git a/Src/exec.c b/Src/exec.c
index fb2739a..1c2a904 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -198,7 +198,8 @@ static char *blank_env[] = { NULL };
 /* Execution functions. */
 
 static int (*execfuncs[WC_COUNT-WC_CURSH]) _((Estate, int)) = {
-    execcursh, exectime, execfuncdef, execfor, execselect,
+    execcursh, exectime, NULL /* execfuncdef handled specially */,
+    execfor, execselect,
     execwhile, execrepeat, execcase, execif, execcond,
     execarith, execautofn, exectry
 };
@@ -1116,8 +1117,11 @@ execsimple(Estate state)
 	    fflush(xtrerr);
 	}
 	lv = (errflag ? errflag : cmdoutval);
-    } else
+    } else if (code == WC_FUNCDEF) {
+	lv = execfuncdef(state, NULL);
+    } else {
 	lv = (execfuncs[code - WC_CURSH])(state, 0);
+    }
 
     thisjob = otj;
 
@@ -2790,6 +2794,42 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	return;
     }
 
+    if (type == WC_FUNCDEF) {
+	/*
+	 * The first word of a function definition is a list of
+	 * names.  If this is empty, we're doing an anonymous function:
+	 * in that case redirections are handled normally.
+	 * If not, it's a function definition: then we don't do
+	 * redirections here but pass in the list of redirections to
+	 * be stored for recall with the function.
+	 */
+	if (*state->pc != 0) {
+	    /* Nonymous, don't do redirections here */
+	    redir = NULL;
+	}
+    } else if (is_shfunc) {
+	Shfunc shf = (Shfunc)hn;
+	/*
+	 * A function definition may have a list of additional
+	 * redirections to apply, so retrieve it.
+	 */
+	if (shf->redir) {
+	    struct estate s;
+	    LinkList redir2;
+
+	    s.prog = shf->redir;
+	    s.pc = shf->redir->prog;
+	    s.strs = shf->redir->strs;
+	    redir2 = ecgetredirs(&s);
+	    if (!redir)
+		redir = redir2;
+	    else {
+		while (nonempty(redir2))
+		    addlinknode(redir, ugetnode(redir2));
+	    }
+	}
+    }
+
     if (type == WC_SIMPLE && !nullexec) {
 	char *s;
 	char trycd = (isset(AUTOCD) && isset(SHINSTDIN) &&
@@ -3238,7 +3278,33 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		flags |= ESUB_REVERTPGRP;
 	    entersubsh(flags);
 	}
-	if (type >= WC_CURSH) {
+	if (type == WC_FUNCDEF) {
+	    Eprog redir_prog;
+	    if (!redir && wc_code(*beg) == WC_REDIR)  {
+		/*
+		 * We're not using a redirection from the currently
+		 * parsed environment, which is what we'd do for an
+		 * anonymous function, but there are redirections we
+		 * should store with the new function.
+		 */
+		struct estate s;
+
+		s.prog = state->prog;
+		s.pc = beg;
+		s.strs = state->prog->strs;
+
+		/*
+		 * The copy uses the wordcode parsing area, so save and
+		 * restore state.
+		 */
+		lexsave();
+		redir_prog = eccopyredirs(&s);
+		lexrestore();
+	    } else
+		redir_prog = NULL;
+	    
+	    lastval = execfuncdef(state, redir_prog);
+	} else if (type >= WC_CURSH) {
 	    if (last1 == 1)
 		do_exec = 1;
 	    lastval = (execfuncs[type - WC_CURSH])(state, do_exec);
@@ -4235,7 +4301,7 @@ exectime(Estate state, UNUSED(int do_exec))
 
 /**/
 static int
-execfuncdef(Estate state, UNUSED(int do_exec))
+execfuncdef(Estate state, Eprog redir_prog)
 {
     Shfunc shf;
     char *s = NULL;
@@ -4305,6 +4371,7 @@ execfuncdef(Estate state, UNUSED(int do_exec))
 	shf->node.flags = 0;
 	shf->filename = ztrdup(scriptfilename);
 	shf->lineno = lineno;
+	shf->redir = redir_prog;
 	shfunc_set_sticky(shf);
 
 	if (!names) {
@@ -4335,6 +4402,8 @@ execfuncdef(Estate state, UNUSED(int do_exec))
 	    ret = lastval;
 
 	    freeeprog(shf->funcdef);
+	    if (shf->redir) /* shouldn't be */
+		freeeprog(shf->redir);
 	    zsfree(shf->filename);
 	    zfree(shf, sizeof(*shf));
 	    break;
diff --git a/Src/hashtable.c b/Src/hashtable.c
index ef18792..7a43062 100644
--- a/Src/hashtable.c
+++ b/Src/hashtable.c
@@ -887,6 +887,8 @@ freeshfuncnode(HashNode hn)
     zsfree(shf->node.nam);
     if (shf->funcdef)
 	freeeprog(shf->funcdef);
+    if (shf->redir)
+	freeeprog(shf->redir);
     zsfree(shf->filename);
     if (shf->sticky) {
 	if (shf->sticky->n_on_opts)
@@ -954,10 +956,19 @@ printshfuncnode(HashNode hn, int printflags)
 		printf(" \"$@\"");
 	    }
 	}   
-	printf("\n}\n");
+	printf("\n}");
     } else {
-	printf(" () { }\n");
+	printf(" () { }");
     }
+    if (f->redir) {
+	t = getpermtext(f->redir, NULL, 1);
+	if (t) {
+	    zputs(t, stdout);
+	    zsfree(t);
+	}
+    }
+
+    putchar('\n');
 }
 
 /**************************************/
diff --git a/Src/parse.c b/Src/parse.c
index 3633417..6cba050 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -375,6 +375,8 @@ init_parse(void)
 
 /* Build eprog. */
 
+/* careful: copy_ecstr is from arg1 to arg2, unlike memcpy */
+
 static void
 copy_ecstr(Eccstr s, char *p)
 {
@@ -386,24 +388,25 @@ copy_ecstr(Eccstr s, char *p)
 }
 
 static Eprog
-bld_eprog(void)
+bld_eprog(int heap)
 {
     Eprog ret;
     int l;
 
     ecadd(WCB_END());
 
-    ret = (Eprog) zhalloc(sizeof(*ret));
+    ret = heap ? (Eprog) zhalloc(sizeof(*ret)) : (Eprog) zalloc(sizeof(*ret));
     ret->len = ((ecnpats * sizeof(Patprog)) +
 		(ecused * sizeof(wordcode)) +
 		ecsoffs);
     ret->npats = ecnpats;
-    ret->nref = -1;		/* Eprog is on the heap */
-    ret->pats = (Patprog *) zhalloc(ret->len);
+    ret->nref = heap ? -1 : 1;
+    ret->pats = heap ? (Patprog *) zhalloc(ret->len) :
+	(Patprog *) zshcalloc(ret->len);
     ret->prog = (Wordcode) (ret->pats + ecnpats);
     ret->strs = (char *) (ret->prog + ecused);
     ret->shf = NULL;
-    ret->flags = EF_HEAP;
+    ret->flags = heap ? EF_HEAP : EF_REAL;
     ret->dump = NULL;
     for (l = 0; l < ecnpats; l++)
 	ret->pats[l] = dummy_patprog1;
@@ -455,7 +458,7 @@ parse_event(void)
         clear_hdocs();
         return NULL;
     }
-    return bld_eprog();
+    return bld_eprog(1);
 }
 
 /**/
@@ -534,7 +537,7 @@ parse_list(void)
 	yyerror(0);
 	return NULL;
     }
-    return bld_eprog();
+    return bld_eprog(1);
 }
 
 /*
@@ -553,7 +556,7 @@ parse_cond(void)
         clear_hdocs();
 	return NULL;
     }
-    return bld_eprog();
+    return bld_eprog(1);
 }
 
 /* This adds a list wordcode. The important bit about this is that it also
@@ -2554,6 +2557,73 @@ ecgetredirs(Estate s)
     return ret;
 }
 
+/*
+ * Copy the consecutive set of redirections in the state at s.
+ * Return NULL if none, else an Eprog consisting only of the
+ * redirections from permanently allocated memory.
+ *
+ * s is left in the state ready for whatever follows the redirections.
+ */
+
+/**/
+Eprog
+eccopyredirs(Estate s)
+{
+    Wordcode pc = s->pc;
+    wordcode code = *pc;
+    int ncode, ncodes = 0, r, type;
+
+    if (wc_code(code) != WC_REDIR)
+	return NULL;
+
+    init_parse();
+
+    while (wc_code(code) == WC_REDIR) {
+	type = WC_REDIR_TYPE(code);
+
+	DPUTS(type == REDIR_HEREDOC || type == REDIR_HEREDOCDASH,
+	      "unexpanded here document");
+
+	if (WC_REDIR_FROM_HEREDOC(code))
+	    ncode = 5;
+	else
+	    ncode = 3;
+	if (WC_REDIR_VARID(code))
+	    ncode++;
+	pc += ncode;
+	ncodes += ncode;
+	code = *pc;
+    }
+    r = ecused;
+    ecispace(r, ncodes);
+
+    code = *s->pc;
+    while (wc_code(code) == WC_REDIR) {
+	s->pc++;
+
+	ecbuf[r++] = code;
+	/* fd1 */
+	ecbuf[r++] = *s->pc++;
+	/* name or HERE string */
+	/* No DUP needed as we'll copy into Eprog immediately below */
+	ecbuf[r++] = ecstrcode(ecgetstr(s, EC_NODUP, NULL));
+	if (WC_REDIR_FROM_HEREDOC(code))
+	{
+	    /* terminator, raw */
+	    ecbuf[r++] = ecstrcode(ecgetstr(s, EC_NODUP, NULL));
+	    /* terminator, munged */
+	    ecbuf[r++] = ecstrcode(ecgetstr(s, EC_NODUP, NULL));
+	}
+	if (WC_REDIR_VARID(code))
+	    ecbuf[r++] = ecstrcode(ecgetstr(s, EC_NODUP, NULL));
+
+	code = *s->pc;
+    }
+
+    /* bld_eprog() appends a useful WC_END marker */
+    return bld_eprog(0);
+}
+
 /**/
 mod_export struct eprog dummy_eprog;
 
@@ -3546,4 +3616,3 @@ dump_autoload(char *nam, char *file, int on, Options ops, int func)
     }
     return ret;
 }
-
diff --git a/Src/signals.c b/Src/signals.c
index cb2b581..74a39e5 100644
--- a/Src/signals.c
+++ b/Src/signals.c
@@ -752,7 +752,7 @@ dosavetrap(int sig, int level)
 	Shfunc shf, newshf = NULL;
 	if ((shf = (Shfunc)gettrapnode(sig, 1))) {
 	    /* Copy the node for saving */
-	    newshf = (Shfunc) zalloc(sizeof(*newshf));
+	    newshf = (Shfunc) zshcalloc(sizeof(*newshf));
 	    newshf->node.nam = ztrdup(shf->node.nam);
 	    newshf->node.flags = shf->node.flags;
 	    newshf->funcdef = dupeprog(shf->funcdef, 0);
diff --git a/Src/zsh.h b/Src/zsh.h
index 8f56fa2..d284c7a 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1139,6 +1139,7 @@ struct shfunc {
     char *filename;             /* Name of file located in */
     zlong lineno;		/* line number in above file */
     Eprog funcdef;		/* function definition    */
+    Eprog redir;                /* redirections to apply */
     Emulation_options sticky;   /* sticky emulation definitions, if any */
 };
 
diff --git a/Test/A04redirect.ztst b/Test/A04redirect.ztst
index 7ad02db..436ae59 100644
--- a/Test/A04redirect.ztst
+++ b/Test/A04redirect.ztst
@@ -54,7 +54,7 @@
 0:Here-documents stripping tabs
 >barbar
 
-  cat <<-$'$HERE '`$(THERE) `'$((AND)) '"\EVERYWHERE"
+  cat <<-$'$HERE '`$(THERE) `'$((AND)) '"\EVERYWHERE" #
 # tabs again.  sorry about the max miller.
 	Here's a funny thing.  Here is a funny thing.
 	I went home last night.  There's a funny thing.
@@ -455,3 +455,39 @@
 
   [</dev/null ]
 1:check behaviour with square brackets
+
+  print any old rubbish >input1
+  () {
+    local var
+    read var
+    print I just read $var
+  } <input1 >output1
+  print Nothing output yet
+  cat output1
+0:anonymous function redirections are applied immediately
+>Nothing output yet
+>I just read any old rubbish
+
+  redirfn() {
+    local var
+    read var
+    print I want to tell you about $var
+    print Also, this might be an error >&2
+  } <input2 >output2 2>&1
+  print something I heard on the radio >input2
+  redirfn
+  print No output until after this
+  cat output2
+0:redirections with normal function definition
+>No output until after this
+>I want to tell you about something I heard on the radio
+>Also, this might be an error
+
+  which redirfn
+0:text output of function with redirections
+>redirfn () {
+>	local var
+>	read var
+>	print I want to tell you about $var
+>	print Also, this might be an error >&2
+>} < input2 > output2 2>&1

pws



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