$pipestatus broken

$pipestatus broken

[Oliver Kiddle]

The pipestatus array seems to be broken:

% ls|cat -|cat - >/dev/null
% echo $pipestatus
0

% false
% echo $pipestatus
0

Seems to occur on both branches. I've not really used pipestatus before
so perhaps I'm missing something.

Oliver

Re: $pipestatus broken

[Bart Schaefer]

On Nov 2, 12:23pm, Oliver Kiddle wrote:
}
} The pipestatus array seems to be broken:
} 
} % ls|cat -|cat - >/dev/null
} % echo $pipestatus
} 0

It works with "zsh -f" in the latest dev version:

zagzig% true | false |true 
zagzig% echo $pipestatus
0 1 0

It fails if there's a precmd function defined:

zagzig% precmd() { echo PRECMD: $pipestatus }
PRECMD: 0
zagzig% echo $pipestatus
0
PRECMD: 0
zagzig% false | echo $pipestatus
0
PRECMD: 1 0
zagzig% echo $pipestatus
0
PRECMD: 0

I think there's code to save and restore `lastval' around precmd(), but
it doesn't save and restore the entire `pipestats' array.

Re: $pipestatus broken

[Peter Stephenson]

Bart Schaefer wrote:
> I think there's code to save and restore `lastval' around precmd(), but
> it doesn't save and restore the entire `pipestats' array.

It's in doshfunc.  It would be more efficient to save in restore in
preprompt(), but at the expense of extra code.

I think the first chunk is some debugging code which should have been
deleted.  I can guess who put that there.

There still doesn't seem to be a dupmem function...

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.54
diff -u -r1.54 exec.c
--- Src/exec.c	29 Oct 2003 19:17:30 -0000	1.54
+++ Src/exec.c	3 Nov 2003 11:18:23 -0000
@@ -3064,8 +3064,6 @@
 	if (!out)
 	{
 	    addproc(pid, NULL, 1);
-	    fprintf(stderr, "Proc %d added\n", pid);
-	    fflush(stderr);
 	}
 	return pnam;
     }
@@ -3448,7 +3446,8 @@
  * was executed.                                            */
 {
     char **tab, **x, *oargv0;
-    int oldzoptind, oldlastval, oldoptcind;
+    int oldzoptind, oldlastval, oldoptcind, oldnumpipestats;
+    int *oldpipestats = NULL;
     char saveopts[OPT_SIZE], *oldscriptname = scriptname, *fname = dupstring(name);
     int obreaks;
     struct funcstack fstack;
@@ -3463,6 +3462,16 @@
     if (trapreturn < 0)
 	trapreturn--;
     oldlastval = lastval;
+    oldnumpipestats = numpipestats;
+    if (noreturnval) {
+	/*
+	 * Easiest to use the heap here since we're bracketed
+	 * immediately by a pushheap/popheap pair.
+	 */
+	size_t bytes = sizeof(int)*numpipestats;
+	oldpipestats = (int *)zhalloc(bytes);
+	memcpy(oldpipestats, pipestats, bytes);
+    }
 
     starttrapscope();
 
@@ -3568,8 +3577,11 @@
 
     if (trapreturn < -1)
 	trapreturn++;
-    if (noreturnval)
+    if (noreturnval) {
 	lastval = oldlastval;
+	numpipestats = oldnumpipestats;
+	memcpy(pipestats, oldpipestats, sizeof(int)*numpipestats);
+    }
     popheap();
 
     if (exit_pending) {



-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
The information transmitted is intended only for the person or
entity to which it is addressed and may contain confidential 
and/or privileged material. 
Any review, retransmission, dissemination or other use of, or
taking of any action in reliance upon, this information by 
persons or entities other than the intended recipient is 
prohibited. 
If you received this in error, please contact the sender and 
delete the material from any computer.
**********************************************************************

Re: $pipestatus broken

[Oliver Kiddle]

Bart wrote:
> It fails if there's a precmd function defined:

As it happens, I don't have a precmd function defined. Peter's patch
didn't help: it seems the problem for my setup is down to zle widgets. In
particular, I have one defined for the enter key as a wrapper around
accept-line. $status seems unaffected so it looks like we need to be
saving pipestats somewhere else too.

Oliver

Re: $pipestatus broken

[Peter Stephenson]

Oliver Kiddle wrote:
> As it happens, I don't have a precmd function defined. Peter's patch
> didn't help: it seems the problem for my setup is down to zle widgets. In
> particular, I have one defined for the enter key as a wrapper around
> accept-line. $status seems unaffected so it looks like we need to be
> saving pipestats somewhere else too.

Is this any good?  I haven't properly tested it since you're more in a
position to see if it fixes things.

The widget code wasn't using the save-return-values facilities of
doshfunc because it wanted the status.  I've made doshfunc pass that
back separately.

Index: Src/exec.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/exec.c,v
retrieving revision 1.55
diff -u -r1.55 exec.c
--- Src/exec.c	3 Nov 2003 11:33:46 -0000	1.55
+++ Src/exec.c	3 Nov 2003 13:01:25 -0000
@@ -3436,17 +3436,22 @@
     return shf;
 }
 
-/* execute a shell function */
+/*
+ * execute a shell function
+ *
+ * If noreturnval is nonzero, then reset the current return
+ * value (lastval) to its value before the shell function
+ * was executed.  However, in any case return the status value
+ * from the function (i.e. if noreturnval is not set, this
+ * will be the same as lastval).
+ */
 
 /**/
-mod_export void
+mod_export int
 doshfunc(char *name, Eprog prog, LinkList doshargs, int flags, int noreturnval)
-/* If noreturnval is nonzero, then reset the current return *
- * value (lastval) to its value before the shell function   *
- * was executed.                                            */
 {
     char **tab, **x, *oargv0;
-    int oldzoptind, oldlastval, oldoptcind, oldnumpipestats;
+    int oldzoptind, oldlastval, oldoptcind, oldnumpipestats, ret;
     int *oldpipestats = NULL;
     char saveopts[OPT_SIZE], *oldscriptname = scriptname, *fname = dupstring(name);
     int obreaks;
@@ -3577,6 +3582,7 @@
 
     if (trapreturn < -1)
 	trapreturn++;
+    ret = lastval;
     if (noreturnval) {
 	lastval = oldlastval;
 	numpipestats = oldnumpipestats;
@@ -3599,6 +3605,8 @@
 	    zexit(exit_pending >> 1, 0);
 	}
     }
+
+    return ret;
 }
 
 /* This finally executes a shell function and any function wrappers     *
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.36
diff -u -r1.36 zle_main.c
--- Src/Zle/zle_main.c	29 Oct 2003 19:17:48 -0000	1.36
+++ Src/Zle/zle_main.c	3 Nov 2003 13:01:25 -0000
@@ -910,7 +910,7 @@
 	    zsfree(msg);
 	    ret = 1;
 	} else {
-	    int osc = sfcontext, osi = movefd(0), olv = lastval;
+	    int osc = sfcontext, osi = movefd(0);
 	    int oxt = isset(XTRACE);
 	    LinkList largs = NULL;
 
@@ -924,10 +924,8 @@
 	    makezleparams(0);
 	    sfcontext = SFC_WIDGET;
 	    opts[XTRACE] = 0;
-	    doshfunc(w->u.fnnam, prog, largs, shf->flags, 0);
+	    ret = doshfunc(w->u.fnnam, prog, largs, shf->flags, 1);
 	    opts[XTRACE] = oxt;
-	    ret = lastval;
-	    lastval = olv;
 	    sfcontext = osc;
 	    endparamscope();
 	    lastcmd = 0;

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR Ltd., Science Park, Milton Road,
Cambridge, CB4 0WH, UK                          Tel: +44 (0)1223 692070


**********************************************************************
This email and any files transmitted with it are confidential and
intended solely for the use of the individual or entity to whom they
are addressed. If you have received this email in error please notify
the system manager.

This footnote also confirms that this email message has been swept by
MIMEsweeper for the presence of computer viruses.

www.mimesweeper.com
**********************************************************************

Re: $pipestatus broken

[Oliver Kiddle]

Peter wrote:

> Is this any good?  I haven't properly tested it since you're more in a
> position to see if it fixes things.

That now works. Thanks

> The widget code wasn't using the save-return-values facilities of
> doshfunc because it wanted the status.  I've made doshfunc pass that
> back separately.

Looking at it, that seems sensible.

Would be good to have these applied to 4.0 too.

Thanks

Oliver